Point tango to git rev to fix paired benchmarks - #645
Conversation
`tango` recently switched to running the paired comparison benchmarks in separate processes (bazhenov/tango#78). This is fortunate, as it turns out to also fix an issue I encountered. On current `main`, if I run `cargo export target/benchmarks -- bench --bench=main` followed by `cargo bench -q --bench=main -- compare target/benchmarks/main -p -t 4.`, I get the following! ``` Default Style - arabic 20 characters [ 8.9 us ... 8.9 us ] +0.35% Default Style - latin 20 characters [ 4.3 us ... 4.1 us ] -3.50%* Default Style - japanese 20 characters [ 8.2 us ... 8.1 us ] -0.44% Default Style - arabic 1 paragraph [ 48.5 us ... 48.1 us ] -0.66% Default Style - latin 1 paragraph [ 17.2 us ... 16.1 us ] -6.52%* Default Style - japanese 1 paragraph [ 71.3 us ... 69.6 us ] -2.50%* Default Style - arabic 4 paragraph [ 214.5 us ... 212.7 us ] -0.86% Default Style - latin 4 paragraph [ 66.0 us ... 62.0 us ] -5.96%* Default Style - japanese 4 paragraph [ 100.7 us ... 98.5 us ] -2.18%* Styled - arabic 20 characters [ 9.9 us ... 9.9 us ] +0.41% Styled - latin 20 characters [ 5.3 us ... 5.3 us ] -1.01%* Styled - japanese 20 characters [ 8.8 us ... 8.7 us ] -0.60%* Styled - arabic 1 paragraph [ 51.3 us ... 51.0 us ] -0.60% Styled - latin 1 paragraph [ 21.4 us ... 20.9 us ] -2.52%* Styled - japanese 1 paragraph [ 78.7 us ... 77.5 us ] -1.52%* Styled - arabic 4 paragraph [ 232.7 us ... 232.1 us ] -0.27% Styled - latin 4 paragraph [ 83.2 us ... 80.6 us ] -3.18%* Styled - japanese 4 paragraph [ 109.7 us ... 108.3 us ] -1.34%* ``` This is without any code changes and isn't measurement noise. A possible explanation is there's a difference between the in-process code and the `dlopen`'d code performance that shows up at least on my machine. I don't think it's a P- vs E-core thing or something like that, as when I pin the threads to equally-powerful cores (same features, same max freq), the results stay like this. Measured with this PR, the results are more reasonable. ``` $ cargo export target/benchmarks -- bench --bench=main $ cargo bench -q --bench=main -- compare ../target/benchmarks/main -t 4. Default Style - arabic 20 characters [ 8.7 us ... 8.7 us ] -0.04% Default Style - latin 20 characters [ 4.1 us ... 4.1 us ] +1.76%* Default Style - japanese 20 characters [ 7.9 us ... 8.0 us ] +0.24% Default Style - arabic 1 paragraph [ 46.7 us ... 46.9 us ] +0.47% Default Style - latin 1 paragraph [ 16.0 us ... 16.0 us ] -0.08% Default Style - japanese 1 paragraph [ 68.4 us ... 68.3 us ] -0.10% Default Style - arabic 4 paragraph [ 196.5 us ... 196.9 us ] +0.23% Default Style - latin 4 paragraph [ 61.1 us ... 61.1 us ] +0.10% Default Style - japanese 4 paragraph [ 96.7 us ... 96.4 us ] -0.32% Styled - arabic 20 characters [ 9.6 us ... 9.6 us ] +0.19% Styled - latin 20 characters [ 5.2 us ... 5.3 us ] +0.32% Styled - japanese 20 characters [ 8.5 us ... 8.4 us ] -0.10% Styled - arabic 1 paragraph [ 49.1 us ... 49.0 us ] -0.08% Styled - latin 1 paragraph [ 20.4 us ... 20.4 us ] -0.05% Styled - japanese 1 paragraph [ 74.2 us ... 74.4 us ] +0.22% Styled - arabic 4 paragraph [ 216.0 us ... 215.8 us ] -0.10% Styled - latin 4 paragraph [ 79.1 us ... 79.2 us ] +0.19% Styled - japanese 4 paragraph [ 105.2 us ... 105.4 us ] +0.17% ```
|
|
||
| # Compare changes with baseline | ||
| cargo bench -q --bench=main -- compare target/benchmarks/main -p | ||
| cargo bench -q --bench=main -- compare ../target/benchmarks/main |
There was a problem hiding this comment.
The target is now relative to current_dir, see bazhenov/tango#73. The -p flag to bench in separate threads is dropped as I believe tango now unconditionally spawns two processes.
There was a problem hiding this comment.
Does this mean we need to have cding into this directory as a step?
(Edit: maybe not? Seems like this is something which cargo sets; this seems a bit jank...)
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
This makes me want to try running this in CI...
| $ cargo install cargo-export | ||
| ``` | ||
|
|
||
| NOTE: Windows users may require additional setup. See Tango docs for more information. |
There was a problem hiding this comment.
Does this Tango update maintain this caveat?
There was a problem hiding this comment.
Yep, https://github.com/bazhenov/tango/tree/1d69f0728b707597409be0cc199e7a0173a666dd#getting-started: Windows requires rustflags = ["-Zexport-executable-symbols"].
There was a problem hiding this comment.
Hey! 👋 Tango maintainer here. After merging bazhenov/tango#78 -Zexport-executable-symbols is actually not needed. Today I've merged the code that checks that everything works without any custom configuration on a stable toolchain on all 3 platforms (see bazhenov/tango#80).
So you can remove it as well as nightly toolchain on Windows and build.rs modifications on Linux/macOS. Cheers!
There was a problem hiding this comment.
Awesome, thank you for notifying us and for maintaining tango!
#646 removes the old custom config from Parley.
|
|
||
| # Compare changes with baseline | ||
| cargo bench -q --bench=main -- compare target/benchmarks/main -p | ||
| cargo bench -q --bench=main -- compare ../target/benchmarks/main |
There was a problem hiding this comment.
Does this mean we need to have cding into this directory as a step?
(Edit: maybe not? Seems like this is something which cargo sets; this seems a bit jank...)
Tango now works without options/build scripts, see bazhenov/tango#80. Denis, the maintainer of Tango, kindly notified us of this in linebender#645 (comment)! (The bump is not strictly necessary I believe, but ensures our config is synchronized with the README of the `tango` rev we point to.)
Tango now works without options/build scripts, see bazhenov/tango#80. Denis, maintainer of Tango, kindly notified us of this in linebender#645 (comment)! (The bump is not strictly necessary I believe, but ensures our config is synchronized with the README of the `tango` rev we point to.)
3 participants
tangorecently switched to running the paired comparison benchmarks in separate processes (bazhenov/tango#78). This is fortunate, as it turns out to also fix an issue I encountered in #634 (comment).On current
main, if I runcargo export target/benchmarks -- bench --bench=mainfollowed bycargo bench -q --bench=main -- compare target/benchmarks/main -p -t 4., I get the following!I.e., there are quite a few significant "improvements." This is without any code changes and isn't measurement noise. A possible explanation is there's a difference between the in-process code and the
dlopen'd code performance that shows up at least on my machine. I don't think it's a P- vs E-core thing or something like that, as when I pin the threads to equally-powerful cores (same features, same max freq), the results stay like this.Measured using the git revision of
tango, the results are more reasonable.