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

cruft: parallelize globs matching #13

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Nov 9, 2022

The most expensive operation of cruft is the filtering of the existing paths with the configured filters and excludes. Perform that task in parallel by using C++17 parallel algorithms. One downside is, since the standard library does not implement this functionality itself, this requires linking against tbb (Threading Building Blocks).

Sequenced:

Benchmark 1: ./cruft -E ./explain/ -F ./rules/ -I ./ignore -R ./ruleset
  Time (mean ± σ):      7.179 s ±  0.079 s    [User: 6.366 s, System: 2.519 s]
  Range (min … max):    7.060 s …  7.291 s    10 runs

Parallel:

Benchmark 1: ./cruft -E ./explain/ -F ./rules/ -I ./ignore -R ./ruleset
  Time (mean ± σ):      4.762 s ±  0.058 s    [User: 6.659 s, System: 2.414 s]
  Range (min … max):    4.682 s …  4.849 s    10 runs

@a-detiste
Copy link
Owner

Can you rebase ? Looks good.

The most expensive operation of cruft is the filtering of the existing
paths with the configured filters and excludes.  Perform that task in
parallel by using C++17 parallel algorithms.  One downside is, since the
standard library does not implement this functionality itself, this
requires linking against tbb (Threading Building Blocks).

Sequenced:

    Benchmark 1: ./cruft -E ./explain/ -F ./rules/ -I ./ignore -R ./ruleset
      Time (mean ± σ):      7.179 s ±  0.079 s    [User: 6.366 s, System: 2.519 s]
      Range (min … max):    7.060 s …  7.291 s    10 runs

Parallel:

    Benchmark 1: ./cruft -E ./explain/ -F ./rules/ -I ./ignore -R ./ruleset
      Time (mean ± σ):      4.762 s ±  0.058 s    [User: 6.659 s, System: 2.414 s]
      Range (min … max):    4.682 s …  4.849 s    10 runs
@a-detiste
Copy link
Owner

thanks

@a-detiste
Copy link
Owner

I need to test this on Buster

@cgzones
Copy link
Contributor Author

cgzones commented Jun 8, 2023

While rerunning the benchmark I did not notice a significant time saving anymore. Maybe the simplification of myglob() made this obsolete.

@a-detiste
Copy link
Owner

a-detiste commented Jun 11, 2023

It looks like a case of too early optimisation on both sides.
It was a big mistake on my side to optimize for a match, I should have instead optimized for a non match (what happens 99% of the time).

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