Add HashMap.adjustWithKey.

[m-renaud]

This function is present in the container's Map interface, this brings more
consistency to the HashMap interface so that it can more easily be used as a
drop in replacement for Data.Map if desired.

This partially addresses #172.

[treeowl]

Let's start with the most general: alterF. We can think about others later. I suspect the implementation should be similar to the one for IntMap, but you should be sure to calculate the hash only once.

[m-renaud]

Sounds good, it looks like I'm going to have to pull out the go functions for lookup, insert and delete into top level helpers (so that I can pass the computed hash value into them instead of them computing it each time). Does that sound reasonable?

[treeowl]

Sounds likely!

…

On Jan 24, 2018 12:52 AM, "Matt Renaud" ***@***.***> wrote: Sounds good, it looks like I'm going to have to pull out the go functions for lookup, insert and delete into top level helpers (so that I can pass the computed hash value into them instead of them computing it each time). Does that sound reasonable? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_bdm23aLiPQLtYOk7ovy9CwQy9Vxks5tNsUsgaJpZM4RqvLf> .

[treeowl]

You'll probably need to do a bit more than that, actually. You want to avoid testing key equality more than once. I think that means you'll need a version of lookup that records the key position in case of collision, and specialized versions of insert and delete to support the following situations: * insert, knowing that the key is not yet in the map * insert, knowing that the key is in the map without collision * insert, knowing the key is in the map with a particular position in a collision * delete, knowing the key is in the map without collision * delete, knowing the key is in the map in a particular collision location. You should be able to merge the functions for present keys together, passing a bogus (negative) index when there was no collision.

…

On Jan 24, 2018 12:54 AM, "David Feuer" ***@***.***> wrote: Sounds likely! On Jan 24, 2018 12:52 AM, "Matt Renaud" ***@***.***> wrote: > Sounds good, it looks like I'm going to have to pull out the go > functions for lookup, insert and delete into top level helpers (so that > I can pass the computed hash value into them instead of them computing it > each time). Does that sound reasonable? > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#178 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABzi_bdm23aLiPQLtYOk7ovy9CwQy9Vxks5tNsUsgaJpZM4RqvLf> > . >

[treeowl]

Also, we're not adding adjustWithKey. I'd much rather *remove* it from containers. It's a fussy micro-optimization and I just don't buy it.

…

On Jan 24, 2018 1:47 AM, "David Feuer" ***@***.***> wrote: You'll probably need to do a bit more than that, actually. You want to avoid testing key equality more than once. I think that means you'll need a version of lookup that records the key position in case of collision, and specialized versions of insert and delete to support the following situations: * insert, knowing that the key is not yet in the map * insert, knowing that the key is in the map without collision * insert, knowing the key is in the map with a particular position in a collision * delete, knowing the key is in the map without collision * delete, knowing the key is in the map in a particular collision location. You should be able to merge the functions for present keys together, passing a bogus (negative) index when there was no collision. On Jan 24, 2018 12:54 AM, "David Feuer" ***@***.***> wrote: > Sounds likely! > > On Jan 24, 2018 12:52 AM, "Matt Renaud" ***@***.***> wrote: > >> Sounds good, it looks like I'm going to have to pull out the go >> functions for lookup, insert and delete into top level helpers (so that >> I can pass the computed hash value into them instead of them computing it >> each time). Does that sound reasonable? >> >> — >> You are receiving this because you commented. >> Reply to this email directly, view it on GitHub >> <#178 (comment)>, >> or mute the thread >> <https://github.com/notifications/unsubscribe-auth/ABzi_bdm23aLiPQLtYOk7ovy9CwQy9Vxks5tNsUsgaJpZM4RqvLf> >> . >> >

[m-renaud]

we're not adding adjustWithKey

SGTM.

version of lookup that records the key position in case of collision

Sounds reasonable.

insert, knowing that the key is not yet in the map

I don't see how these would allow us to cut out any operations, you'd still need to traverse through the layers of BitmapIndexed and Full until you reach an Empty. Am I missing something?

insert, knowing that the key is in the map without collision

I can see this, you can skip the ky == k check of the Leaf case.

insert, knowing the key is in the map with a particular position in a collision

Yup, makes sense, just jump to the right index in the collision array

delete, knowing the key is in the map without collision

If I'm reading the implementation right, if we know that a key k is in the map without collision then we will reach a Leaf constructor once, and that leaf must contain the key that we're deleting. Is that correct?

delete, knowing the key is in the map in a particular collision location.

Yup, same as for insert, just jump to the right index in the collision array

[treeowl]

When you insert knowing the key is not in the map, you don't have to test equality if you find that its *hash* is in the map. These versions of insert and delete won't need Eq constraints.

…

On Jan 24, 2018 6:42 PM, "Matt Renaud" ***@***.***> wrote: we're not adding adjustWithKey SGTM. version of lookup that records the key position in case of collision Sounds reasonable. insert, knowing that the key is not yet in the map I don't see how these would allow us to cut out any operations, you'd still need to traverse through the layers of BitmapIndexed and Full until you reach an Empty. Am I missing something? insert, knowing that the key is in the map without collision I can see this, you can skip the ky == k check of the Leaf case. insert, knowing the key is in the map with a particular position in a collision Yup, makes sense, just jump to the right index in the collision array delete, knowing the key is in the map without collision If I'm reading the implementation right, if we know that a key k is in the map without collision then we will reach a Leaf constructor once, and that leaf *must* contain the key that we're deleting. Is that correct? delete, knowing the key is in the map in a particular collision location. Yup, same as for insert, just jump to the right index in the collision array — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_cZmJw3fl30FonEGf3OBtArmdYEbks5tN7_sgaJpZM4RqvLf> .

[m-renaud]

So. Much. Code. Duplication. :P

[treeowl]

Be grateful it's easier than the Data.Map version!

…

On Jan 24, 2018 8:05 PM, "Matt Renaud" ***@***.***> wrote: So. Much. Code. Duplication. :P — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_d3W76JL3cQvdtbBt0OFlBToXzmJks5tN9NSgaJpZM4RqvLf> .

[treeowl]

Not adding adjustWithKey; alterF was added in #181.