Skip to content

perf: compare_perf sign and sort order disagree with their comments #84

Description

@kirillDevPro

What looks wrong

crates/moon-perf/src/implementation.rs documents three orderings that the code does not implement. This was noticed while looking for a pure function to test; the behaviour was not encoded as a test, because a test would turn the disagreement into a contract.

Output::compare_perf

The doc comment says a positive PerfReport means self performed better than baseline. The shift is baseline_iters_per_sec / self_iters_per_sec - 1, which is negative when self is faster. PerfReport's display then draws an up arrow for a positive shift, so a slower run is shown as an improvement.

Output::sort

The comment says tests with no metadata go at the end. The comparator returns Greater when the left row has metadata and the right row does not, so rows without metadata sort first.

Output's display

The comment says important tests should print at the top. sort orders Importance ascending (Fluff = 0 ... Critical = 4) and the display walks that order, so the least important rows come first.

Why this is an issue and not a fix

The daily test lane does not change production behaviour. Whichever reading is intended (the comments or the code) needs a human decision before a test pins it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions