Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

cmd/unity: gather performance stats #37

Open
myitcv opened this issue Mar 3, 2021 · 0 comments
Open

cmd/unity: gather performance stats #37

myitcv opened this issue Mar 3, 2021 · 0 comments
Labels
enhancement New feature or request
Milestone
Next

Comments

@myitcv
Copy link
Contributor

myitcv commented Mar 3, 2021

Currently, unity tests simply assert that testscript-based tests pass or fail their assertions. This obviously gives us correctness confidence with respect to changes made to CUE's evaluation engine.

However, it does not give us any sort of steer on how a given change has/hasn't impacted performance.

Time-based benchmarking is tricky because it is very sensitive to collateral noise on the machine where tests are being run. One alternative approach is to emit CUE-specific performance statistics like "number of unifications" or "number of disjunctions evaluated" and use that as a proxy for performance.

This issue is therefore about two things:

  • working out what kind of statistics cmd/cue and the various CUE APIs could record
  • having unity collect those statistics for a given test run and presenting the results in some way (a "significant" performance regression could count as a test fail for example)

cc @mpvl
@myitcv myitcv added the enhancement New feature or request label Mar 3, 2021
@myitcv myitcv added this to the Next milestone Mar 3, 2021
@mvdan mvdan mentioned this issue Sep 22, 2022
Closed
cueckoo pushed a commit to cue-lang/cue-trybot that referenced this issue Feb 17, 2023
@mvdan
cue/stats: use int64 for stats counters
a339e39 
This ensures consistent behavior between 32-bit and 64-bit GOARCHes,
and makes overflows in methods like Add less likely.

While here, add a TODO note about overflows and underflows.

For cue-unity/unity#37.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Iac3ca73815a79e6df550c5c4b9a13d894047e25d
@cueckoo cueckoo mentioned this issue Feb 17, 2023
Closed
cueckoo pushed a commit to cue-lang/cue that referenced this issue Feb 20, 2023
@mvdan
cue/stats: use int64 for stats counters
7073554 
This ensures consistent behavior between 32-bit and 64-bit GOARCHes,
and makes overflows in methods like Add less likely.

While here, add a TODO note about overflows and underflows.

For cue-unity/unity#37.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Iac3ca73815a79e6df550c5c4b9a13d894047e25d
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/549849
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
porcuepine pushed a commit that referenced this issue Feb 20, 2023
@mvdan
cmd/cue: include evaluator stats in test output via CUE_STATS_FILE
d2e7d9b 
Running the command

	$ unity test --unsafe --skip-base commit:9a7c8fb04e74bc29a6b93507bbfd56be682431d7 commit:7d89acd5c44bca0c756f76089436ee9913818b03

on cue-unity-example with two CUE master commits (the current tip and
its parent), unity used to print:

	testing github.com/cue-unity/example against version 9a7c8fb04e74bc29a6b93507bbfd56be682431d7
	testing github.com/cue-unity/example against version 7d89acd5c44bca0c756f76089436ee9913818b03
	ok  github.com/cue-unity/example  9a7c8fb04e74bc29a6b93507bbfd56be682431d7  0.030s
	ok  github.com/cue-unity/example  7d89acd5c44bca0c756f76089436ee9913818b03  0.031s  +1.337%  9a7c8fb04e74bc29a6b93507bbfd56be682431d7

And now prints:

	testing github.com/cue-unity/example against version 9a7c8fb04e74bc29a6b93507bbfd56be682431d7
	testing github.com/cue-unity/example against version 7d89acd5c44bca0c756f76089436ee9913818b03
	ok  github.com/cue-unity/example  9a7c8fb04e74bc29a6b93507bbfd56be682431d7
	    WallTime                      0.033s
	    Unifications                  6
	    Disjuncts                     6
	    Conjuncts                     6
	    Freed                         6
	    Reused                        0
	    Allocs                        6
	    Retained                      0
	ok  github.com/cue-unity/example  7d89acd5c44bca0c756f76089436ee9913818b03  vs 9a7c8fb04e74bc29a6b93507bbfd56be682431d7
	    WallTime                      0.033s                                    -0.940%
	    Unifications                  6                                         ~
	    Disjuncts                     6                                         ~
	    Conjuncts                     6                                         ~
	    Freed                         6                                         ~
	    Reused                        0                                         ~
	    Allocs                        6                                         ~
	    Retained                      0                                         ~

The result is far more verbose, but there are seven more metrics to
report per result, so they wouldn't all fit in a single line.

This approach continues to use the tablewriter,
but using the table fields for different purposes depending on the line.
For each result, we start with the result summary,
and then a number of indented (via an empty first field) continuation
lines for each of the metrics.
For each of those lines, an extra field is added if we're comparing
against a "first" version.

Right now these are only printed out, they don't affect whether "unity
test" succeeds or fails on a particular CUE change.

Updates #37.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I5bae21bd02f59011b5621d0f98ef3cab37a16dcd
Reviewed-on: https://review.gerrithub.io/c/cue-unity/unity/+/549758
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
TryBot-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
enhancement New feature or request
Milestone
Next
Development

No branches or pull requests
1 participant
@myitcv