Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Casl 534 java proxy fix #347

Merged
merged 4 commits into from
Sep 26, 2024
Merged

Casl 534 java proxy fix #347

merged 4 commits into from
Sep 26, 2024

Conversation

Amaneusz
Copy link
Collaborator

No description provided.

Signed-off-by: Jakub Amanowicz <jakub.amanowicz@here.com>
val constructor: KFunction<T> = primaryNonArgConstructorOf(klass)
?: firstNonArgConstructorOf(klass)
?: throw IllegalArgumentException("Unable to find primary or non-arg constructor for class: ${klass.qualifiedName}")
proxy = constructor.call()
Copy link
Member

@xeus2001 xeus2001 Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, any constructor that does not require arguments is fine for us, we should always first try the primary constructor, but if that does not match, try to find any other constructor.
These checks and searching for the correct constructor can be time consuming, I would suggest to add a cache here, a concurrent hash-map, to cache the found constructors. Like:

var constructor = cache[klass]
if (constructor == null) {
  constructor = ... find the correct one
  cache[klass] = constructor
}

We do not need any synchronization here, because we can assume that the finding operation is idempotent, so even when multiple threads in parallel enter the search and override each other, the cache should work after that as intended.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Signed-off-by: Jakub Amanowicz <jakub.amanowicz@here.com>
Signed-off-by: Jakub Amanowicz <jakub.amanowicz@here.com>
@@ -512,11 +512,21 @@ actual class Platform {
if (klass.isInstance(proxy)) return proxy as T
if (doNotOverride) throw IllegalStateException("The symbol $symbol is already bound to incompatible type")
}
proxy = klass.primaryConstructor!!.call()

proxy = nonArgConstructorFor(klass).call()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the caching, should be just a few code lines, but avoids to always go through reflection and search through constructor lists.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Signed-off-by: Jakub Amanowicz <jakub.amanowicz@here.com>
@Amaneusz Amaneusz merged commit 400303b into v3 Sep 26, 2024
2 of 3 checks passed
@Amaneusz Amaneusz deleted the CASL-534_java_as_proxy branch September 26, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants