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

First stab at auto downcast feat for @Adapter annotation #542

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nyholku
Copy link

@nyholku nyholku commented Jan 16, 2022

This is a draft pull request for an automatic downcast feature for JavaCPP

@nyholku
Copy link
Author

nyholku commented Jan 16, 2022

@saudet

Continuating discussion from here:

#538 (reply in thread)

I agree that Java side solution is much better for the end users, so I will work on that.

The reason I initially did this in the way I did it was that I thought this was more in the spirit of JavaCPP because all the calls that JavaCPP creates seem to be native and also that it is theoretically/marginally slightly more effective to do this:

        std::string klassName = std::string("occ/TKernel$")+std::string(p->DynamicType()->Name());

on the C++ side avoiding JNI calls for Dynamic_Type() and Name() and perhaps doing the string concatenation with a fixed buffer and strcpy.

The class loop up should be cached and that might be trivially more efficient on the C++ side, IDK.

Maybe that was premature optimisation.

Perhaps we could/should have both options (native side and Java side auto downcast)?

@saudet
Copy link
Member

saudet commented Jan 16, 2022

It's not even "premature optimisation", JNI upcalls are an order of magnitude slower than downcalls. Your approach is both slower and less user friendly. It's totally completely pointless. If you don't believe me, benchmark it yourself!

@nyholku
Copy link
Author

nyholku commented Jan 16, 2022

Ok, I did not know that. Thanks for educating me.

@saudet
Copy link
Member

saudet commented Jan 17, 2022

What would make sense to do on the native side is some hack that doesn't use reflection or anything like that. The only thing that I can think of that is remotely efficient would be using something like std::type_index() to get an index into a table of Java classes. That's code that could be fully generated that wouldn't require any input from users, other than a flag to indicate they'd like to have that inefficient code generated, but I'm not sure it's worth the effort. If you'd like to try though, please do!

@nyholku
Copy link
Author

nyholku commented Jan 17, 2022

That is an idea if one would looking for performance. But I don't think it is worth the effort when there is no potential users/use case in sight. I'm looking at doing on Java side basically what I do on C++ side now. Hope to have some time to devout to this next weekend.

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