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

Get rid of TruffleBoundary in Map.insert and Map.get #5233

Closed
4 of 8 tasks
wdanilo opened this issue Feb 5, 2023 · 1 comment · Fixed by #8425
Closed
4 of 8 tasks

Get rid of TruffleBoundary in Map.insert and Map.get #5233

wdanilo opened this issue Feb 5, 2023 · 1 comment · Fixed by #8425
Assignees
Labels
-compiler p-low Low priority x-new-feature Type: new feature request

Comments

@wdanilo
Copy link
Member

wdanilo commented Feb 5, 2023

This task is automatically imported from the old Task Issue Board and it was originally created by Pavel Marek.
Original issue is here.


A follow-up of https://www.pivotaltracker.com/n/projects/2539304/stories/181027272 - the previous story got bigger than we expected, so we have decided to split it.

EnsoHashMapBuilder.get is behind TruffleBoundary, because a black listed method is reachable from it. It seems that the usage of org.graalvm.collections.EconomicMap was not a good idea for a hash map implementation after all. I partially increased the performance by adding @TruffleBoundary(allowInline = true) on some places (bc953f9). Try to investigate the performance more, and ultimately, get rid of TruffleBoundary in Map.insert and Map.get altogether.

Tasks:

  • Consider computing hashCode when computing == on Atom objects
  • Functions shall not be comparable (only use "same object policy" as in NaN can be used as a key in Map #6301)
  • NaNs aren't suitable keys as they aren't supposed to be comparable to themselves - done by NaN can be used as a key in Map #6301
  • Demonstrate effective implementation of distinct via Map (for objects that don't provide custom == & co.)
  • Show how to use the Enso Map from polyglot Java
  • Investigate the performance of EnsoHashMap (details in the description)

For future

Blockers:

https://www.pivotaltracker.com/n/projects/2539304/stories/181027272 resolved

@jdunkerley jdunkerley removed the -libs Libraries: New libraries to be implemented label Feb 6, 2023
@jdunkerley jdunkerley added this to the Beta Release milestone Feb 7, 2023
@Akirathan Akirathan linked a pull request Feb 10, 2023 that will close this issue
4 tasks
@jdunkerley jdunkerley modified the milestone: Beta Release Feb 21, 2023
@JaroslavTulach JaroslavTulach changed the title HashMap/HashSet follow-up Get rid of TruffleBoundary in Map.insert and Map.get Apr 11, 2023
@JaroslavTulach
Copy link
Member

Brainstorming idea. The issue

needs a partial evaluation friendly Map (or Set) of already attached warnings. It'd be great if we could share the same implementation with EnsoHashMap. At the end the needs of Vector.distinct and filtering of "warning duplicates" seem to be very similar.

@jdunkerley jdunkerley removed this from the Beta Release milestone Jul 11, 2023
@mergify mergify bot closed this as completed in #8425 Dec 1, 2023
mergify bot pushed a commit that referenced this issue Dec 1, 2023
Fixes #5233 by removing `EconomicMap` & co. and using plain old good _linear hashing_. Fixes #8090 by introducing `StorageEntry.removed()` rather than copying the builder on each removal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-compiler p-low Low priority x-new-feature Type: new feature request
Projects
Archived in project
4 participants