Skip to content

Fix quadratic COW copies in DictionaryOfDictionaries #654

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
May 12, 2021

Conversation

harlanhaskins
Copy link
Contributor

@harlanhaskins harlanhaskins commented May 12, 2021

DictioanryOfDictionaries implemented its subscript in terms of updateValue
and removeValue, both of which keep track of and return the old value, and then
threw away the old value.

Instead, flip it so removeValue and updateValue are implemented in terms of the
subscript, and make the subscript modify the dictionary in-place.

This ended up being a 700x speedup on an Intel Mac and a 180x speedup on an M1 for a particular stress test.

Fixes rdar://77882768

DictioanryOfDictionaries implemented its subscript in terms of updateValue
and removeValue, both of which keep track of and return the old value, and then
threw away the old value.

Instead, flip it so removeValue and updateValue are implemented in terms of the
subscript, and make the subscript modify the dictionary in-place.

This ended up being a 700x speedup on an Intel Mac and a 180x speedup on an M1.

Fixes rdar://77882768
@CodaFi
Copy link
Contributor

CodaFi commented May 12, 2021

@swift-ci test

@CodaFi CodaFi merged commit 3d2a616 into main May 12, 2021
@CodaFi CodaFi deleted the a-perfectly-spherical-cow branch May 12, 2021 06:14
@davidungar
Copy link
Contributor

This ended up being a 700x speedup on an Intel Mac and a 180x speedup on an M1 for a particular stress test.

Great numbers! How where they measured? Overall driver time?? Just removal of an entry? If the latter, how much overall driver speedup?

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