Skip to content

Conversation

@sjakobi
Copy link
Member

@sjakobi sjakobi commented Oct 22, 2025

Closes #399.


TODO:

  • Run the full benchmarks and compare the results
  • Remove bitmapIndexedOrFull
  • Remove mentions of Full
  • Remove Full-specific functions
  • Update util/Stats.hs

@sjakobi
Copy link
Member Author

sjakobi commented Oct 22, 2025

First impressions:

  • The code size reductions are mostly moderate, except with union and intersection where they are quite enormous.
  • fromList gets slightly slower
  • lookup seems to get slightly faster (~5%) in maps that didn't have a Full constructor to begin with. But for large maps I'm seeing slowdowns of up to ~15% (All.HashMap.Strict.lookup.presentKey.Int.10000: 13 ns -> 15 ns).

@treeowl
Copy link
Collaborator

treeowl commented Oct 22, 2025

Very nice! Did you check to make sure you haven't broken an assumption anywhere that a BitmapIndexed node isn't full? That's what intimidated me about this particular project.

@treeowl
Copy link
Collaborator

treeowl commented Oct 22, 2025

I'm surprised by the lookup regression. Any guesses about why?

@sjakobi
Copy link
Member Author

sjakobi commented Oct 22, 2025

Did you check to make sure you haven't broken an assumption anywhere that a BitmapIndexed node isn't full?

I rely on the "valid" property tests. It would probably be a good idea to check the test coverage again (#270).

I'm surprised by the lookup regression. Any guesses about why?

In Full nodes it's very simple to find the right array index:

index :: Hash -> Shift -> Int
index w s = fromIntegral $ unsafeShiftR w s .&. subkeyMask

In BitmapIndexed nodes the computation is more involved:

sparseIndex
:: Bitmap
-- ^ Bitmap of a 'BitmapIndexed' node
-> Bitmap
-- ^ One-bit 'mask' corresponding to the 'index' of a hash
-> Int
-- ^ Index into the array of the 'BitmapIndexed' node
sparseIndex b m = popCount (b .&. (m - 1))
{-# INLINE sparseIndex #-}

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.

Try removing the Full constructor

2 participants