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

improve fidelity of check if atom fits in SmallAtom #384

Merged
merged 4 commits into from
Feb 14, 2024
Merged

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Feb 13, 2024

This factors out some common code and generally simplifies and unifies the behavior. With additional tests and fuzzer checks.

The second commit addresses the use of as_unique_address(), which is used in ObjectCache as a cheap hash map (index into a Vec). However, with the SmallAtom feature, NodePtr indices is no longer dense, and it no longer works to use a Vec instead of a HashMap. It allocates too much memory if you create an atom with value 0x3ffffff.

This commit is included in this PR since the increased fidelity of matching atoms to SmallAtom pushed the fuzzer over the limit and started failing with out-of-memory.

@arvidn arvidn mentioned this pull request Feb 13, 2024
Copy link

coveralls-official bot commented Feb 13, 2024

Pull Request Test Coverage Report for Build 7900735840

Details

  • 0 of 21 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 94.301%

Totals Coverage Status
Change from base Build 7888730470: -0.04%
Covered Lines: 5742
Relevant Lines: 6089

💛 - Coveralls

Rigidity
Rigidity previously approved these changes Feb 13, 2024
Rigidity
Rigidity previously approved these changes Feb 14, 2024
Copy link
Contributor

@Rigidity Rigidity left a comment

Choose a reason for hiding this comment

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

lgtm

@arvidn arvidn mentioned this pull request Feb 14, 2024
@arvidn arvidn merged commit 3ded0d4 into main Feb 14, 2024
29 checks passed
@arvidn arvidn deleted the small-atom-fidelity branch February 14, 2024 21:13
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.

None yet

2 participants