AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
920 stars 329 forks source link

Convert all plugin rezconfig to rezconfig.py #1692

Closed brycegbrazen closed 3 months ago

brycegbrazen commented 4 months ago

Effectively copied over work that @nerdvegas did back on this commit.

Closes #525.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 58.27%. Comparing base (3c75a19) to head (00a18a8). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1692 +/- ## ========================================== + Coverage 58.25% 58.27% +0.01% ========================================== Files 126 126 Lines 17157 17157 Branches 3504 3504 ========================================== + Hits 9995 9998 +3 + Misses 6496 6494 -2 + Partials 666 665 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JeanChristopheMorinPerso commented 3 months ago

Thanks @brycegbrazen. Did you notice a performance different between loading configs from python files instead of YAML? I feel like it'll be slightly faster with python files than with YAML since YAML is hard to parse and hence slow. But we should check to make sure there is no unexpected regression.

We could use https://github.com/sharkdp/hyperfine to test rez config plugins's speed on this PR versus the code we have on main.

BryceGattis commented 3 months ago

@JeanChristopheMorinPerso I didn't benchmark anything at this point, but that does sound like a great idea.

Should rez-benchmark include the ability to run CLI benchmarks like this? Or perhaps we should just set-up a new GitHub CI test that runs like all of our other ones?

brycegbrazen commented 3 months ago

I ran hyperfine before my update (on main):

hyperfine --warmup 3 "rez config plugins"
Benchmark 1: rez config plugins
  Time (mean ± σ):     427.9 ms ±   9.4 ms    [User: 204.7 ms, System: 180.3 ms]
  Range (min … max):   417.7 ms … 449.8 ms    10 runs

And then after the update:

hyperfine --warmup 3 "rez config plugins"
Benchmark 1: rez config plugins
  Time (mean ± σ):     429.7 ms ±  14.6 ms    [User: 217.5 ms, System: 179.1 ms]
  Range (min … max):   412.7 ms … 460.1 ms    10 runs
JeanChristopheMorinPerso commented 3 months ago

Thanks. I also tested on my machine and I see approximately the same stats for both:

Benchmark 1: /tmp/asd/old/bin/rez/rez-config plugins
  Time (mean ± σ):     250.1 ms ±   4.2 ms    [User: 206.3 ms, System: 42.7 ms]
  Range (min … max):   244.1 ms … 260.5 ms    11 runs

Benchmark 2: /tmp/asd/new/bin/rez/rez-config plugins
  Time (mean ± σ):     251.7 ms ±   4.5 ms    [User: 211.3 ms, System: 39.3 ms]
  Range (min … max):   246.0 ms … 262.8 ms    11 runs

Summary
  /tmp/asd/old/bin/rez/rez-config plugins ran
    1.01 ± 0.02 times faster than /tmp/asd/new/bin/rez/rez-config plugins
JeanChristopheMorinPerso commented 3 months ago

(correction, I wrongly installed rez in the previous run, my bad)

Benchmark 1: /tmp/asd/old/bin/rez/rez-config plugins
  Time (mean ± σ):     265.6 ms ±   4.0 ms    [User: 227.4 ms, System: 36.7 ms]
  Range (min … max):   258.2 ms … 270.2 ms    11 runs

Benchmark 2: /tmp/asd/new/bin/rez/rez-config plugins
  Time (mean ± σ):     250.6 ms ±   4.9 ms    [User: 214.5 ms, System: 34.4 ms]
  Range (min … max):   243.8 ms … 258.3 ms    11 runs

Summary
  /tmp/asd/new/bin/rez/rez-config plugins ran
    1.06 ± 0.03 times faster than /tmp/asd/old/bin/rez/rez-config plugins