Skip to content

[Runtime] Shrink ConcurrentReadableHashMap a bit. #33647

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

Merged
merged 1 commit into from
Aug 28, 2020

Conversation

mikeash
Copy link
Contributor

@mikeash mikeash commented Aug 26, 2020

We're using a lot of space on the free lists. Each vector is three words, and we have two of them. Switch to a single linked list. We only need one list, as both kinds of pointers just get free()'d. A linked list optimizes for the common case where the list is empty. This takes us from six words to one.

Also make ReaderCount, ElementCount, and ElementCapacity uint32_ts. The size_ts were unnecessarily large and this saves some space on 64-bit systems.

While we're in there, add 0/NULL initialization to all elements. The current use in the runtime is unaffected (it's statically allocated) but the local variables used in the test were tripping over this.

@mikeash mikeash requested a review from tbkka August 26, 2020 18:34
@mikeash
Copy link
Contributor Author

mikeash commented Aug 26, 2020

@swift-ci please test

Copy link
Contributor

@tbkka tbkka left a comment

Choose a reason for hiding this comment

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

Nice!

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 5ebdd2d65d045a9dda94d4882476203be3310a61

We're using a lot of space on the free lists. Each vector is three words, and we have two of them. Switch to a single linked list. We only need one list, as both kinds of pointers just get free()'d. A linked list optimizes for the common case where the list is empty. This takes us from six words to one.

Also make ReaderCount, ElementCount, and ElementCapacity uint32_ts. The size_ts were unnecessarily large and this saves some space on 64-bit systems.

While we're in there, add 0/NULL initialization to all elements. The current use in the runtime is unaffected (it's statically allocated) but the local variables used in the test were tripping over this.
@mikeash mikeash force-pushed the shrink-concurrentreadablehashmap branch from 5ebdd2d to 4cd1b71 Compare August 27, 2020 17:05
@mikeash
Copy link
Contributor Author

mikeash commented Aug 27, 2020

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 5ebdd2d65d045a9dda94d4882476203be3310a61

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 5ebdd2d65d045a9dda94d4882476203be3310a61

@mikeash
Copy link
Contributor Author

mikeash commented Aug 27, 2020

@swift-ci please test os x platform

@mikeash
Copy link
Contributor Author

mikeash commented Aug 28, 2020

@swift-ci please test linux platform

@mikeash mikeash merged commit 0990fa9 into swiftlang:master Aug 28, 2020
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.

3 participants