[lworld] RFR: 8372515: [lworld] Plumb in javac flags for compiling with preview mode [v2]
Maurizio Cimadamore
mcimadamore at openjdk.org
Wed Dec 10 10:38:01 UTC 2025
On Tue, 25 Nov 2025 15:57:56 GMT, David Beaumont <duke at openjdk.org> wrote:
>> Plumbing for javac flags, mostly inspired by/copied from test commits made by @lahodaj .
>>
>> There are several things here, mostly entangled, so it's a bit tricky to try splitting this out, but it would be possible if people wanted.
>>
>> The biggest "refactoing" part of this PR is "src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java" which now has a properly controlled lifecycle and disposed its resources correctly. Prior to this, the class used a non-closeable JRT file-system reference, which leads to "persistent open file" issues such as JDK-8357249.
>>
>> This *does* mean that if compilation and the runtime have the same preview mode, then a 2nd JRT file system to the same jimage file is "opened", *but* the file system itself is lightweight, non-caching and both of them will use the underlying SharedImageReader (which is where nodes are cached etc.) so it really shouldn't be an issue (I will make sure javac benchmarks are checked however).
>>
>> The benefit of this is that now, the shared index (which does do some caching) is correctly tracked across all users, and will be closed when the last user closes the lightweight wrapper instance.
>>
>> A lot of the smaller "spot fix" changes in this PR were just copied by me, or at least inspired directly by Jan's work, so I may have missed some semantic subtlety in the code I'm not familiar with. Please evaluate that carefully.
>
> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>
> Remove note about StableValue (not possible)
src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java line 88:
> 86:
> 87: // Synchronized by FileSystemResources.class, NOT instance.
> 88: private final Set<JRTIndex> owners = new HashSet<>();
This seems like a ref counter -- e.g. we want to keep track of how many JRTIndex instances there are that point at the same underlying resources. This seems sensible, and doing it this way seems better than using a refcount, so that you can check that the guy who did the close is the same as that who did the open.
Since there's only one set for both modes, correctness currently hinges (I think) on the fact that JRTIndex does *not* override equals/hashCode. This means that, if the same client opens two indices, one for preview, one for non-preview, you will see two distinct entries in the set (because the comparison is an identity comparison).
We might want to leave a comment about this "hidden" dependencies. Or, we could split the owner set in two. If we do the latter, we might improve the "already opened" check -- so that we can say "index already opened _in mode XYZ_"
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2606100908
More information about the valhalla-dev
mailing list