8000 perf: `sort` microoptimize by jqnatividad · Pull Request #2748 · dathere/qsv · GitHub
[go: up one dir, main page]

Skip to content

perf: sort microoptimize #2748

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
May 20, 2025
Merged

perf: sort microoptimize #2748

merged 3 commits into from
May 20, 2025

Conversation

jqnatividad
Copy link
Collaborator
  • qualify how we randomize data (by row)
  • micro-optimize writing sorted data, by pulling flag_unique check outside hot write loop (though IO-bound, no sense repeatedly checking flag_unique inside loop
  • inline always compare_float which is a one-liner

- qualify how we randomize data (by row)
- micro-optimize writing sorted data, by pulling flag_unique check outside hot write loop (though IO-bound, no sense repeatedly checking flag_unique inside loop
- inline always compare_float which is a one-liner
@jqnatividad jqnatividad requested a review from Copilot May 20, 2025 19:45
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR micro-optimizes the sort command by refining documentation, reducing branch checks in the hot loop, and inlining a one-liner comparison function.

  • Clarified the --random flag description.
  • Pulled the flag_unique check outside the record-writing loop.
  • Added #[inline(always)] to compare_float and silenced Clippy’s warning.

@@ -42,7 +42,7 @@ sort options:
When not set, the number of jobs is set to the
number of CPUs detected.
--faster When set, the sort will be faster. This is done by
using a faster sorting algorithm that is not stable
using a faster sorting algorithm that is not "stable"
Copy link
Preview
Copilot AI May 20, 2025 8000

Choose a reason for hiding this comment

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

[nitpick] Wrapping stable in quotes may be confusing; use backticks (stable) to reference the stability guarantee or remove the quotes entirely for consistency with other option descriptions.

Suggested change
using a faster sorting algorithm that is not "stable"
using a faster sorting algorithm that is not `stable`

Copilot uses AI. Check for mistakes.

jqnatividad and others added 2 commits May 20, 2025 16:05
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jqnatividad jqnatividad merged commit c2b2195 into master May 20, 2025
12 of 13 checks passed
@jqnatividad jqnatividad deleted the sort-microoptimize branch May 20, 2025 20:08
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.

1 participant
0