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

ref(debuginfo): Replace dmsort with std sort #869

Merged
merged 7 commits into from
Sep 17, 2024
Merged

Conversation

loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Sep 17, 2024

We're using an unstable sorting algorithm (dmsort) to sort symbols upon collection. It behaves differently under Rust 1.81 than 1.80, so some snapshots change.

@loewenheim loewenheim self-assigned this Sep 17, 2024
@loewenheim loewenheim requested review from a team and Swatinem September 17, 2024 11:01
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.44%. Comparing base (bd90dfb) to head (f62903d).
Report is 31 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #869      +/-   ##
==========================================
+ Coverage   74.45%   77.44%   +2.98%     
==========================================
  Files          63       61       -2     
  Lines       15432    14853     -579     
==========================================
+ Hits        11490    11503      +13     
+ Misses       3942     3350     -592     

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

tbh, this is a bit suspect. though its possibly explained with identical code folding, where multiple debuginfo entries exist for the same (deduplicated) physical address.

as an aside: I don’t think we need a separate dmsort implementation anymore, as std sort should be the fastest kid in town by now.

@loewenheim
Copy link
Contributor Author

loewenheim commented Sep 17, 2024

If we switch to std sort, we might also do a stable sort, wdyt?

@loewenheim loewenheim changed the title fix(tests): Fix test snapshots for 1.81 ref(debuginfo): Replace dmsort with std sort Sep 17, 2024
@loewenheim loewenheim merged commit 7cee3f4 into master Sep 17, 2024
14 checks passed
@loewenheim loewenheim deleted the fix/snapshots branch September 17, 2024 12: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.

3 participants