RFR: 7903649: Field and global variables of array type should have indexed accessors
Jorn Vernee
jvernee at openjdk.org
Tue Jan 30 17:08:41 UTC 2024
On Tue, 30 Jan 2024 16:01:17 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> The above approach can be dialled down in a number of ways:
>> * maybe a `jextract` option can make all these holder classes "private", so that they don't show up in the API
>> * for structs, we might only generate holder classes for array fields (on the basis that other info is quickly obtainable when looking at the struct layout).
>>
>> Stepping back - I think what we need to ask ourselves is whether this incremental complexity of the generated code is worth the added benefits (e.g. of providing more direct access to constants that might be required from the generated code). It might also be useful to take some footprint measurements before/after. It might well be that the extra holder class doesn't matter much in terms of overall bytes of the compiled code. While it's true that there's more classes to load, it's also true that this approach makes everything more lazy - e.g. so that an offset, or a var handle is only computed if the corresponding field is accessed by a client. The two things might well cancel each other out.
>
>> It might also be useful to take some footprint measurements before/after.
> Here are some rough measurements using libpython. There are approximatively 140 struct classes (out of ~240 generated files). Here's what I got:
>
> Size of generated sources
> vanilla: 5.3M
> holder: 5.4M
>
> Size of generated classes
> vanilla: 18M
> holder: 20M
This looks like a good approach to me. I think we mostly care about the sources (esp. now that we always generate sources). The cost in terms of size doesn't seem bad. I'm assuming this is for an unfiltered extraction of libpython. I think our assumption is that most of the time, users that care about size will apply filters as well.
As far as source code itself goes. This doesn't look bad. TBH, I think it visually looks a bit cleaner since the constants are 'bagged up' nicely in a separate class that doesn't take up too much space. (at least at the moment. I think we need javadoc for the constant classes as well).
Considering discoverability, lets say someone wants to access the `x` field. When typing 'x', auto-complete would show 2 overloads of `x`, and then `x$constants`. That doesn't seem too cluttered, and I think it's clear from the name that the constant class is not for the common 'I just want to read `x`' use case, given the `$` in the name. (I think it's actually helping us out here).
-------------
PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1471629774
More information about the jextract-dev
mailing list