Skip to content

Improve Deserialization Performance #734

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 3 commits into from
Jun 30, 2021
Merged

Improve Deserialization Performance #734

merged 3 commits into from
Jun 30, 2021

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Jun 29, 2021

Consume the new non-owning bitstream deserialization in swiftlang/swift-tools-support-core#236 to provide a 3-5x boost to deserialization performance for source and module dependency graph files. Also, undo a cached hash optimization that was only really necessary because of the quadratic COW issue resolved in #654. Otherwise, the hash was being eagerly computed during deserialization which forced an extra half-second of wall time.

CodaFi added 3 commits June 28, 2021 23:04
We only needed this because of quadratic hashing costs that were corrected in swiftlang#655. As it stands, computing this hash up front is pessimizing deserialization of source file dependency graphs.
There's no need to completely empty out this scratch buffer especially since we can use it to amortize the cost of reallocation of the backing buffer for the array here.
Blobs are now decoded as ArraySlices, so we need to switch to the stdlib entrypoint. This has the added upside of being *much* more performant than String.init(data:encoding:) which not only cost a copy to create the Data, but also could not take advantage of stdlib's fast paths for UTF8 buffers as inputs.
@CodaFi CodaFi requested review from davidungar and artemcm June 29, 2021 07:13
Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

All of these changes look great to me.

Copy link
Contributor

@davidungar davidungar left a comment

Choose a reason for hiding this comment

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

It would be nice to add a comment somewhere about the strategy for handling fingerprint strings, which in this PR is designed for performance. Either here, or in a subsequent PR.
But that said, LGTM!

@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 30, 2021

@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 30, 2021

@CodaFi CodaFi merged commit b2a26b5 into swiftlang:main Jun 30, 2021
@CodaFi CodaFi deleted the sysopt branch June 30, 2021 21:30
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