Skip to content

Extend JNI to Managed type map caching - #9818

Closed
simonrozsival wants to merge 1 commit into
mainfrom
dev/simonrozsival/improve-typemap-caching
Closed

Extend JNI to Managed type map caching#9818
simonrozsival wants to merge 1 commit into
mainfrom
dev/simonrozsival/improve-typemap-caching

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

This PR suggests caching "shortcuts" between class names which don't have a direct mapping to a managed class, but their base class has one. This should reduce the need to repeatedly call GetJavaToManagedType and inspect Java type hierarchies.

// cache shortcuts for the class names of the subclasses in the hierarchy we just explored
if (class_names is not null) {
foreach (var subclass_name in class_names) {
TypeManagerMapDictionaries.JniToManaged.Add (subclass_name, type);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the type here actually the correct one? It seems like it would be the final type it found and not the one that matches the class_name earlier in the loop.

Do you need to store a (string, Type) tuple, so you have the previous Type instance?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right that it is not an exact match. On the other hand, by inspecting the superclasses in the type hierarchy we should always arraive at the same type. So my idea was to cache this "shortcut".

The only problem I can think of is if we stored this "shortcut", any later call to TypeManager.RegisterType with a more specific mapping would be ignored. This could break dynamic registration. I will need to give this some more thought.

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants