RFR: 7903586: Revisit jextract constant classes [v2]
Jorn Vernee
jvernee at openjdk.org
Wed Nov 29 11:51:39 UTC 2023
On Wed, 29 Nov 2023 11:27:08 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> This PR removes the use of jextract constant classes, and falls back to a simpler code generation strategies which move the various constants closer to where they belong:
>>
>> * for structs and callbacks, the constants are added to the class/interface being generated;
>> * for the header class, we use a local holder class per function to hold the method handle and function descriptor constants.
>>
>> This scheme results in much less generated code (as no constant class is emitted), but with the same performance and laziness as the old scheme.
>>
>> When working with the code, I've decided to simplify the handling of constant generation, in the spirit of what was recently done with the introduction of string templates. That is, instead of using a magic `Constant` class, which has methods to generate getters/setters for a given constant, I converted much of the code into "static" string templates, so that it's easier to see which code gets generated when staring at the jextract code. As a result, the `Constant` class has been removed.
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>
> More doc alignment fixes
Marked as reviewed by jvernee (Committer).
src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 238:
> 236: incrAlign();
> 237: appendLines(STR."""
> 238: private static final MemorySegment \{mangledName} = RuntimeHelper.lookupGlobalVariable(\"\{nativeName}\", \{layoutString(0, layout)});
Shouldn't this re-use the layout field we emit instead of emitting the same layout string again inline?
src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 258:
> 256: \{MEMBER_MODS} MemoryLayout \{mangledName}() {
> 257: return \{mangledName};
> 258: }
Now that we no longer have the constants classes, I wonder if we should just do away with the getter here and just expose the layout field.
I noticed that we are also already exposing the FunctionDescriptor in a functional interface class as a field, as part of this patch,
-------------
PR Review: https://git.openjdk.org/jextract/pull/145#pullrequestreview-1755032323
PR Review Comment: https://git.openjdk.org/jextract/pull/145#discussion_r1409161492
PR Review Comment: https://git.openjdk.org/jextract/pull/145#discussion_r1409139996
More information about the jextract-dev
mailing list