Skip to content

Conversation

@mbien
Copy link
Member

@mbien mbien commented Oct 27, 2025

first commit fixes the internal reference tracking WeakHashMap:

  • WeakHashMap keys, which are used to track the lifecycle of BaseFileObj, must be identity objects.
  • this can be also seen as a bugfix, since Integers are bad reference strength indicators due to the internal Integer boxing cache - some are always strongly reachable

second commit is for module cleanup

  • language level bump, overrides and simplifications.

tested using:

26-jep401ea2+1-1 (don't forget -J--enable-preview)

NB starts and projects load at least, but more work may be needed in other areas (did only very basic testing).

value object JEP: https://openjdk.org/jeps/401

meta issue #8256

@mbien mbien added Platform [ci] enable platform tests (platform/*) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Oct 27, 2025
@BlueGoliath

This comment was marked as off-topic.

@mbien
Copy link
Member Author

mbien commented Oct 28, 2025

The errors from these issues must have been silently thrown because Netbeans 27 started just fine for me. The IDE did crash everytime the editor's context menu was opened though.

Of course Netbeans was also highlighted perfectly fine code as having errors. I could've sworn removing nb-javac worked around that last EA build but maybe not.

This is not about editor support for JDK 26 or any editor features regarding value types. This PR so far is just here to allow (very) early testing of running NB on ea JDKs which have the JEP 401 implementation enabled.

@mbien mbien force-pushed the valhalla branch 2 times, most recently from 8b5381f to 561766c Compare October 28, 2025 13:21
@mbien mbien added the ci:all-tests [ci] enable all tests label Oct 30, 2025
@mbien mbien changed the title Valhalla Awaits Master Filesystem Valhalla compatibility Oct 30, 2025
@mbien mbien added this to the NB29 milestone Oct 30, 2025
@mbien mbien marked this pull request as ready for review October 30, 2025 06:11
@mbien
Copy link
Member Author

mbien commented Nov 1, 2025

while renovating the code I found more issues in FileObjectFactory but addressing them would be out of scope of this PR.

E.g:

  • the list entry of the internal cache can't shrink atm.
  • there is also an instance identity problem which could cause the WeakHashMap to remove a full List entry even though some of the list items are still alive
  • FileObjectFactory::toString will throw CCE if the cache contained a list (printing all entries might also not be reasonable)

I might do some more cleanup as preparation for the fixes though.

Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible to me.

@mbien
Copy link
Member Author

mbien commented Nov 3, 2025

will rebase on top of #8799 so that we can test both valhalla related updates using the same dev build

planning to integrate this this week, dependent on feedback. Thanks for the review!

mbien added 2 commits November 7, 2025 18:02
The WeakHashMap keys, which are used to track the lifecycle of
BaseFileObj, must be identity objects.

This is also a bugfix, since Integers are bad instances to track
object reachablility due to the internal Integer auto boxing cache
which holds a strong reference to some instances.
Language level bump, overrides and code simplifications.
@mbien
Copy link
Member Author

mbien commented Nov 7, 2025

all green, ran a indexer smoke test which indexed all ~850 nb modules using 26-jep401ea and checked a few things manually (eg some dialogs due to #8799) everything worked as before.

thanks for the review -> merging

@mbien mbien merged commit b8eba3d into apache:master Nov 7, 2025
77 of 80 checks passed
@mbien mbien removed DO NOT squash ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:all-tests [ci] enable all tests Code cleanup Platform [ci] enable platform tests (platform/*)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants