Skip to content

[multimapcache] Add an efficient CRTP based write-once multimap cache that can be small. #30665

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

Conversation

gottesmm
Copy link
Contributor

The properties of this multimap cache are:

  1. Values are stored (inline if Small) in a Vector and our map internally maps
    keys to (start, length) of slices of the Vector. This is done instead of
    storing arrays refs to ensure that if our array goes from small -> large, we
    do not have stale pointers.

  2. Values are only allowed to be inserted all at once. This is ok, since this is
    a cache.

  3. One is not storing individual small vectors in a map (or state storing
    SmallVectors). This can inadvertantly add up to using a lot of memory and is
    not needed for homogenous data.

@gottesmm
Copy link
Contributor Author

@swift-ci test

@gottesmm
Copy link
Contributor Author

@swift-ci test asan

@gottesmm
Copy link
Contributor Author

@swift-ci asan test

@gottesmm gottesmm force-pushed the pr-148512f9e52b5c768ddd3312717c2fffcd71a96e branch from 76ce410 to d21cdea Compare March 31, 2020 03:38
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

… that can be small.

The properties of this multimap cache are:

1. Values are stored (inline if Small) in a Vector and our map internally maps
   keys to (start, length) of slices of the Vector. This is done instead of
   storing arrays refs to ensure that if our array goes from small -> large, we
   do not have stale pointers.

2. Values are only allowed to be inserted all at once. This is ok, since this is
   a cache.

3. One is not storing individual small vectors in a map (or state storing
   SmallVectors). This can inadvertantly add up to using a lot of memory and is
   not needed for homogenous data.
@gottesmm gottesmm force-pushed the pr-148512f9e52b5c768ddd3312717c2fffcd71a96e branch from d21cdea to 00d4576 Compare March 31, 2020 22:13
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

1 similar comment
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm merged commit 8e447a4 into swiftlang:master Apr 1, 2020
@gottesmm gottesmm deleted the pr-148512f9e52b5c768ddd3312717c2fffcd71a96e branch April 1, 2020 01:04
@atrick
Copy link
Contributor

atrick commented Apr 1, 2020

I don't think CRTP buys anything here but it does make the code confusing both here and on the client side.
From the client's perspective this is neither a multi-map nor has any additional caching support beyond a normal map. It's just an array map that happens to be implemented as a flat array. I'd call it something like FlatArrayMap and keep the design simple and self contained.

[EDIT] I read the code more carefully. It is caching a None result, so it is definitely appropriate to call it a cache. Calling it a multi-map is a serious conceptual problem because the client associates a complete array with a single instance of the key. It's critical to understand that the array is all-or-nothing, signifying a complete vs. incomplete analysis!

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