8384611: [lworld] Add value class test coverage to java.util HashMap and HashSet by bwhuang-us · Pull Request #2501 · openjdk/valhalla

bwhuang-us · 2026-06-02T22:00:10Z

Changed Tests:

HashMap/KeySetRemove.java: equal ValueClass keys mapped to null can be removed from HashMap and TreeMap.

HashMap/NullKeyAtResize.java: null-key resize behavior remains correct when surrounding keys are ValueClass instances.

HashMap/PutNullKey.java: colliding comparable tree-bin keys are value-capable via @AsValueClass.

HashMap/ReplaceExisting.java: replacing an existing ValueClass key during active iteration does not corrupt the iterator.

HashMap/SetValue.java: Map.Entry.setValue() returns the old ValueClass value.

HashMap/ToArray.java: toArray() coverage is added for ValueClass keys, values, and set elements across hash and linked variants.

HashMap/TreeBinAssert.java: tree-bin iterator-removal coverage now uses a value-capable key.

Hashtable/EqualsCast.java: Provider/Hashtable equality now includes matching ValueClass key/value entries.

Hashtable/SimpleSerialization.java: serialization round-trip now also covers Hashtable<Integer,Integer>.

LinkedHashMap/ComputeIfAbsentAccessOrder.java: access-order behavior is repeated with Integer keys.

LinkedHashMap/EmptyMapIterator.java: fail-fast iterator behavior is repeated with Integer key/value entries.

LinkedList/AddAll.java: append-order behavior is repeated with ValueClass elements.

LinkedList/Clone.java: clone/equality checks for LinkedList, TreeSet, and TreeMap subclasses now include ValueClass contents.

Unchanged Tests:

HashMap/HashMapCloneLeak.java: unchanged because the test depends on WeakReference reachability. Value objects are not valid weak-reference targets, so adding VClass would not match the regression being tested.

HashMap/OverrideIsEmpty.java: unchanged because the test is about HashMap subclass method dispatch. Value classes cannot extend HashMap, and changing only the key/value payload would not add meaningful value-class coverage.

HashMap/WhiteBoxResizeTest.java: unchanged because it is a white-box/internal capacity and table-sizing test using reflection/VarHandles over HashMap, LinkedHashMap, HashSet, and WeakHashMap internals. Its purpose is sizing/lazy allocation/resize arithmetic, not key/value equality or value-object behavior.

HashMap/ToString.java: unchanged because it specifically verifies that HashMap.Entry.toString() does not throw when the map contains null keys or values. Adding value-class keys or values would not extend the original null-handling regression in a meaningful way.

HashSet/Serialization.java: unchanged because the existing HashSet serialization round trip already provides platform value-class coverage when run under the Valhalla preview configuration.

Hashtable/DeserializedLength.java: unchanged because it already uses Integer keys and values. Under preview, that gives platform value-class coverage while preserving the test’s original focus on deserialized table capacity.

Hashtable/HashCode.java: unchanged because it intentionally tests the hash code of an empty Hashtable. Adding value-class entries would create a different non-empty-map test.

Hashtable/IllegalLoadFactor.java: unchanged because it validates constructor load-factor arguments and does not insert keys or values. Value-class semantics are irrelevant to this regression.

Hashtable/ReadObject.java: unchanged because it tests that deserialization does not call an overridable put method in a Hashtable subclass. Value classes cannot be Hashtable subclasses.

Hashtable/SelfRef.java: unchanged because it tests self-referential mutable identity maps in toString and hashCode. Value objects are not the self-referential identity object under test.

Hashtable/SerializationDeadlock.java: unchanged because it tests synchronization/deadlock behavior during serialization of mutually referential Hashtable identity objects. Adding value-class entries would be incidental.

LinkedHashMap/Basic.java: unchanged because it already uses Integer keys and values throughout, giving broad value-class coverage under preview.

LinkedHashMap/Cache.java: unchanged because it already uses Integer keys, so the cache/eldest-entry behavior is covered with platform value-class keys under preview.

LinkedHashSet/Basic.java: unchanged because the existing Integer element coverage already exercises value-class set behavior under preview, including equality, hashing, cloning, serialization, iteration, and clear behavior.

LinkedList/ComodifiedRemove.java: unchanged because it already uses Integer elements. The fail-fast iterator behavior is therefore covered with platform value-class elements under preview.

LinkedList/Remove.java: unchanged because it already uses an Integer element. The list-iterator remove/add behavior is covered with platform value-class elements under preview.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8384611: [lworld] Add value class test coverage to java.util HashMap and HashSet (Task - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2501/head:pull/2501
$ git checkout pull/2501

Update a local copy of the PR:
$ git checkout pull/2501
$ git pull https://git.openjdk.org/valhalla.git pull/2501/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2501

View PR using the GUI difftool:
$ git pr show -t 2501

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2501.diff

Using Webrev

Link to Webrev Comment

…ections

…ap and HashSet

bridgekeeper · 2026-06-02T22:17:47Z

👋 Welcome back bhuang! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-06-02T22:45:14Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

mlbridge · 2026-06-02T22:53:58Z

Webrevs

01: Full - Incremental (b704f531)
00: Full (0cc66e6c)

lmesnik · 2026-06-03T00:02:32Z

@@ -40,5 +42,12 @@ public static void main(String[] args) throws Exception {
            if (!m[i].keySet().remove("bananas"))
                throw new Exception("Yes, we have no bananas: "+i);
        }
+
+        Map[] valueMaps = {new HashMap<>(), new TreeMap<>()};
+        for (int i=0; i<valueMaps.length; i++) {


please update indentation (spaces)

Just to confirm, are you referring to the spacing inside the for loop parentheses, e.g. changing for (int i=0; i<valueMaps.length; i++) to for (int i = 0; i < valueMaps.length; i++)?

Yes, exactly.

lmesnik · 2026-06-03T00:05:38Z

@@ -45,6 +47,7 @@ public class PutNullKey {
    // A value 1.0 will ensure that a new threshold == capacity
    static final float LOAD_FACTOR = 1.0f;

+    @AsValueClass


Can you please add the second testcase and new class
CollidingHashValue
that is ' @AsValueClass'

You fix is going to work perfectly now, but later the plan is to rewrite @AsValueClass with 'value class' and don't run the test with and without preview. So 2 testcases would be reuiqred.

lmesnik · 2026-06-03T00:07:18Z

        Map m = new HashMap();
        m.put(key, oldValue);
        Map.Entry e = (Map.Entry) m.entrySet().iterator().next();
        Object returnVal = e.setValue(newValue);
        if (!returnVal.equals(oldValue))
            throw new RuntimeException("Return value: " + returnVal);
    }
+
+    private static void testTupleValue() {


Not sure if tuple is good name here, might be testVClassValue?

lmesnik · 2026-06-03T00:36:55Z


    public static void main(String[] args) {
        checkMap(false);
        checkMap(true);
        checkSet(false);
        checkSet(true);
+        checkTupleMap(false);


Same about tuple in name.
"Not sure if tuple is good name here, might be testVClassValue?"

lmesnik · 2026-06-03T00:37:55Z

@@ -154,6 +156,7 @@ private void doTest(Object collection, int[] hashes,
    /**
     * Class that will have specified hash code in a HashMap.
     */
+    @AsValueClass


The same comment about having 2 testcases.

bwhuang-us added 9 commits May 7, 2026 16:21

JDK-83883658 [lworld] Add value class test coverage to java.util.Coll…

a2c8542

…ections

Update valhalla_adopted test group

f81ee61

Revert AddAll test iteration to 100

9d395b1

JDK-8384611 [lworld] Add value class test coverage to java.util HashM…

fc57bfc

…ap and HashSet

JDK-8384611 [lworld] Add value class test coverage to java.util HashM…

ddb2d77

…ap and HashSet

Update typo in TEST.groups

56e6fe2

Removed Tuple.java from previous commit

26d3307

Update copyright headers

62ef28e

Rename function name

0cc66e6

openjdk Bot changed the title ~~JDK-8384611 [lworld] Add value class test coverage to java.util HashMap and HashSet~~ 8384611: [lworld] Add value class test coverage to java.util HashMap and HashSet Jun 2, 2026

openjdk Bot added the rfr Pull request is ready for review label Jun 2, 2026

lmesnik suggested changes Jun 3, 2026

View reviewed changes

Implemented review comments

b704f53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8384611: [lworld] Add value class test coverage to java.util HashMap and HashSet#2501

8384611: [lworld] Add value class test coverage to java.util HashMap and HashSet#2501
bwhuang-us wants to merge 10 commits into
openjdk:lworldfrom
bwhuang-us:hashmap

bwhuang-us commented Jun 2, 2026 •

edited by openjdk Bot

Loading

Uh oh!

bridgekeeper Bot commented Jun 2, 2026

Uh oh!

openjdk Bot commented Jun 2, 2026

Uh oh!

mlbridge Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

bwhuang-us Jun 3, 2026

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

lmesnik Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bwhuang-us commented Jun 2, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper Bot commented Jun 2, 2026

Uh oh!

openjdk Bot commented Jun 2, 2026

Uh oh!

mlbridge Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

bwhuang-us commented Jun 2, 2026 •

edited by openjdk Bot

Loading

mlbridge Bot commented Jun 2, 2026 •

edited

Loading