fix: numeric median sort + show sub-ms time precision in charts#97
Merged
Conversation
values.sort() defaults to lexicographic string order, which produces a wrong median as soon as a metric has 2+ digit values. E.g. [12, 9, 3] would sort to ["12", "3", "9"] and report median = 3 instead of 9.
Pairs with rspack web-infra-dev/rspack#14049 which emits sub-ms logger.time values; before this change the chart axis/tooltip rounded those to integer ms and hid the µs-scale precision the data carries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two small precision fixes for benchmark stats and display:
calcStatisticsmedian is wrong for multi-digit values.values.sort()defaults to lexicographic string order, so e.g.[12, 9, 3]sorts to["12", "3", "9"]and reportsmedian = 3instead of9.min/maxare unaffected because they useMath.min/Math.max.Chart formatTime truncates ms display to integer. Pairs with rspack fix(stats): preserve sub-millisecond precision in logger time entries rspack#14049 which makes
logger.timeemit sub-millisecond floats — without this change the chart axis and tooltip would still render154 msinstead of154.382 ms, hiding the µs-scale precision the data now carries.Before / After