Skip to content
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

Use IndexMap for performance boost in qubit allocation #215

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented Jan 24, 2025

This change pulls in the IndexMap struct from https://github.com/microsoft/qsharp/tree/main/compiler/qsc_data_structures to use for the qubit id mapping, rather than FxHashMap. In most use cases qubit ids tend to be dense and sequential, making IndexMap an optimal fit. This also cuts down on the execution time of the test_large_state unit test by an order of magnitude, and the new benchmark shows significant gains as well. Other benchmaks showed a small decrease in the perf (around 5%) but since those are measured in microseconds it seems to be a worthwile trade-off (for example, the X gate benchmark was slower by about 0.15 microsecods).

This change pulls in the `IndexMap` struct from https://github.com/microsoft/qsharp/tree/main/compiler/qsc_data_structures to use for the qubit id mapping, rather than `FxHashMap`. In most use cases qubit ids tend to be dense and sequential, making `IndexMap` an optimal fit. This also cuts down on the execution time of the `test_large_state` unit test by an order of magnitude, and the new benchmark shows significant gains as well. Other benchmaks showed a small decrease in the perf (around 5%) but since those are measured in microseconds it seems to be a worthwile trade-off (for example, the X gate benchmark was slower by about 0.15 microsecods).
@swernli swernli requested review from idavis and billti as code owners January 24, 2025 20:25
@swernli
Copy link
Collaborator Author

swernli commented Jan 24, 2025

Flamegraphs from the unit test before the change:
image
after the change:
image

New benchmark perf:

Allocate-Release 10k qubits
                        time:   [180.63 ms 181.10 ms 181.60 ms]
                        change: [-85.182% -85.143% -85.104%] (p = 0.00 < 0.05)
                        Performance has improved.

@swernli swernli merged commit 78fb773 into main Jan 24, 2025
12 checks passed
@swernli swernli deleted the swernli/use-index-map branch January 24, 2025 23:49
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