Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: if the max ordinal for a map value is zero then reserve 1 bit for it instead of nothing #713

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eduardoramirez
Copy link
Contributor

@eduardoramirez eduardoramirez commented Dec 19, 2024

Currently, when a Map is written where all the keys reference the same value ordinal and that ordinal is zero then we don't reserve any space for the value ordinal. when a map entry is hashed into the last bucket, it overflows when trying to set the value in the data array.

@@ -134,7 +134,7 @@ private void gatherStatistics() {
}

bitsPerKeyElement = 64 - Long.numberOfLeadingZeros(maxKeyOrdinal + 1);
bitsPerValueElement = 64 - Long.numberOfLeadingZeros(maxValueOrdinal);
bitsPerValueElement = maxValueOrdinal == 0 ? 1 : 64 - Long.numberOfLeadingZeros(maxValueOrdinal);
Copy link
Contributor

@Sunjeet Sunjeet Dec 20, 2024

Choose a reason for hiding this comment

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

Repro

    @Test
    public void triggerBugWithCurrentHashImpl() {
    ...    
        producer.runCycle((state) -> {
            for (int i=0; i<16384; i++) {  // exactly 16384 entries in the map, or a multiple thereof
                final long id = new Long(i);
                state.add(
                        new MapOfMovies(
                            new HashMap<Long, MyMovie>() {{
                                put(id, new MyMovie(0, "The Matrix")); // all entries pointing to same object at ordinal 0
                }}));
            }
        });
    }

@Sunjeet Sunjeet force-pushed the fix-map-write-state branch from 4e204c7 to 400e645 Compare December 20, 2024 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants