-
Notifications
You must be signed in to change notification settings - Fork 39
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
Experimental Bindgen for Chicory modules #506
Conversation
// Act | ||
var ctxAddr = opa.opaEvalCtxNew(); | ||
var input = "{\"user\": \"alice\"}"; | ||
var inputStrAddr = opa.opaMalloc(input.length()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*/ | ||
@Parameter( | ||
required = true, | ||
defaultValue = "${project.build.directory}/generated-sources/chciory-bindgen") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo "chciory"
I now understand what you're doing here: generate a Java class containing the exports from a WASM file. This givens you a low-level C-like interface that directly matches the WASM interface. That's not bad, and it's useful, better than writing the code by hand. What I have in mind is different: an idiomatic Java interface. This is similar to the annotation support for host modules. Doing this requires additional, higher level information about the WASM interface. The most direct way is to write the Java interface by hand, providing the additional information as annotations. For a large WASM interface, it might be useful to have a code generator that you can run once to get you started, then you can edit it to clean things up. However, with GitHub Copilot or similar AI assistants, the generator might not be needed. I'll use your OPA example to show what I'm envisioning here. |
This has been a big pain point(for me) when integrating multiple different modules.
From my experience, I think that having something completely automatic able to verify at compile time the compatibility of the module API with what has been implemented in Java is instrumental to enable easy upgrades and maintainability. I have been writing this "low level" code enough times now, and is super error-prone and boring to write and maintain. |
I'm still digesting this, but some thoughts on first look. I think this is a lower level concept than what I'd call bindings so i'm not sure if the name is right. There are two layers to bindings to me, the API layer and the ABI layer. This targets the ABI side and leaves you to write the API side. But even then it doesn't quite handle the ABI layer for you. This is fine though as there are lots of potential ABIs we need to support. My one complaint other than the name, and maybe I'm missing something, is I'm not sure this should be completely generated. It feels like it would be nicer to spell it out in a class myself that's in my codebase then you somehow bind the module to that by maybe peppering in some decorators to tell the wasm instance how to bind to it. |
Not attached to it in any way, happy to receive proposals!
At the moment, I'm generating an Let's discuss further how to proceed on this subject, but this PR can easily turn into an |
This is going to be superseded by work in the annotation processing module. |
This should be ready for review, I turned it to use #496 and simplified where possible.
Given the fact that the API of Chicory is not stable, the only reasonable place to develop bindgens is this very same repository, we can revisit this decision in the future.
Happy to hear feedback!
(note #510 should be merged first)old text:
Opening in draft as I noticed that some work done here is partially overlapping with #496 .
Looking for early feedback, especially from @electrum and @evacchi .
For the reviewers, this test shows the expected user experience: https://github.com/dylibso/chicory/compare/main...andreaTP:bindgen?expand=1#diff-6ae6c6d43974944561891d7bfc45e572fca4e97c37547b393527d5ad08ba97f0R15-R70 it can be a little better, but it's in the right direction, wdyt?
Where to go next?
I can move this to the (upcoming)annotation processor or I can roll it out as a Maven plugin, in the latter case I think we should start to refactor some of the logic in a shared library.
Notes:
toHostFunctions
can probably be refactored with Add experimental annotation framework for functions #496