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

Replace std::list with small vector for dependency tracking #100

Merged
merged 2 commits into from
Mar 3, 2022

Conversation

PeterTh
Copy link
Contributor

@PeterTh PeterTh commented Feb 28, 2022

This replaces the use of std::list for dependency tracking with gch::small_vector, and introduces some microbenchmarks for it.

The only other active use of std::list in celerity is in the buffer transfer manager, but all operations on those also seem to involve the network eventually, so micro-optimizing that doesn't appear to make much sense.

Microbenchmark results in graphical form are available here.

@PeterTh PeterTh force-pushed the list-refactor branch 4 times, most recently from 9ff8e8a to b94a753 Compare March 1, 2022 08:45
@PeterTh
Copy link
Contributor Author

PeterTh commented Mar 1, 2022

Note: as per to our previous team discussion, this should probably not be squashed, since it has the old benchmark results in the prior commit and the new ones with the change in the new commit.

@PeterTh PeterTh self-assigned this Mar 1, 2022
@PeterTh PeterTh requested review from BlackMark29A and psalz March 1, 2022 09:53
Copy link

@BlackMark29A BlackMark29A left a comment

Choose a reason for hiding this comment

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

LGTM.

test/benchmarks.cc Outdated Show resolved Hide resolved
test/benchmarks.cc Show resolved Hide resolved
test/benchmarks.cc Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
.gitmodules Show resolved Hide resolved
test/benchmarks.cc Outdated Show resolved Hide resolved
test/benchmarks.cc Show resolved Hide resolved
@PeterTh PeterTh force-pushed the list-refactor branch 3 times, most recently from 7e8fff4 to c34ec43 Compare March 2, 2022 13:32
test/benchmarks.cc Show resolved Hide resolved
Copy link
Member

@psalz psalz left a comment

Choose a reason for hiding this comment

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

LGTM!

@PeterTh PeterTh merged commit c9dab18 into master Mar 3, 2022
@psalz psalz deleted the list-refactor branch March 3, 2022 09:40
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.

4 participants