Skip to content

expr: Get rid of clap for performance reasons#7352

Merged
RenjiSann merged 1 commit intouutils:mainfrom
RenjiSann:expr-no-clap
Feb 25, 2025
Merged

expr: Get rid of clap for performance reasons#7352
RenjiSann merged 1 commit intouutils:mainfrom
RenjiSann:expr-no-clap

Conversation

@RenjiSann
Copy link
Collaborator

@RenjiSann RenjiSann commented Feb 24, 2025

As a side effect, we also now have the right behavior when adding arguments after --help or --version. Before this change, clap would have printed the help, but it is expected that flags are ignored if argc > 2 (more than 1 arg is given)

Benchmarks:

$ export N=2500

# Before
$ hyperfine -m 1000 -- "./expr_clap $(yes 1 | head -n $N | paste -s - 2>&1 | sed 's/\t/ + /g')"
  Time (mean ± σ):      47.4 ms ±   3.3 ms    [User: 34.4 ms, System: 12.8 ms]
  Range (min … max):    37.3 ms …  74.4 ms    1000 runs

#After
$ hyperfine -m 1000 -- "./expr_no_clap $(yes 1 | head -n $N | paste -s - 2>&1 | sed 's/\t/ + /g')"
  Time (mean ± σ):      24.7 ms ±   2.2 ms    [User: 13.9 ms, System: 10.7 ms]
  Range (min … max):    18.8 ms …  34.7 ms    1000 runs

sylvestre
sylvestre reviewed Feb 24, 2025
View reviewed changes
tertsdiepraam
tertsdiepraam reviewed Feb 24, 2025
View reviewed changes
Copy link
Collaborator

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

Cool! Here's a small suggestion

@github-actions
Copy link

github-actions bot commented Feb 24, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Feb 24, 2025

by running the GNU tests manually, I realized that I forgot to handle -- which can appear in the command. I made the necessary changes (diff)

@github-actions
Copy link

github-actions bot commented Feb 24, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

@github-actions
Copy link

github-actions bot commented Feb 25, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@RenjiSann RenjiSann merged commit 9c0ff86 into uutils:main Feb 25, 2025
65 checks passed
@RenjiSann RenjiSann mentioned this pull request Feb 25, 2025
Closed
@moonfruit moonfruit mentioned this pull request Mar 10, 2025
Closed
@cakebaker cakebaker mentioned this pull request Mar 28, 2025
Closed
@BenWiederhake BenWiederhake mentioned this pull request Apr 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments

@tertsdiepraam tertsdiepraam tertsdiepraam left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RenjiSann @sylvestre @tertsdiepraam