Repository navigation
tp: add cold sorts by key for sorts which are not hot - #7710
Conversation
🎨 Perfetto UI Builds & Tests
|
33b2d57 to
5e2f876
Compare
Vibhu-Agarwal
left a comment
There was a problem hiding this comment.
The index-sort + captureless-thunk trick is neat. I went through all the call sites asking "how often, and with how many elements"; all look cold except the two interval-intersect ones (question inline).
Two things on the description:
- It lists the symbolizer among the moved sites, but there's no symbolizer change in the diff.
- Since the motivation is size, the Wasm delta might be a good number to quote, as that's where TP's code size is felt most.
5e2f876 to
fe2294e
Compare
|
Thanks! Rather than reverting the interval intersect sites, I made ColdSort allocation-free for small inputs, so a 3-element sort is now 13ns vs 6ns for |
fe2294e to
8b5cb9a
Compare
std::sort and std::stable_sort are compiled for every element type and
comparator, at 3-4KB each, even where the sort runs once per query or
per trace on a handful of elements.
ColdSortByKey and ColdStableSortByKey sort by an integer key through one
std::sort of {key, index} pairs, compiled once. Each caller adds a few
hundred bytes. Comparing integers inline keeps them within 2x of
std::sort, and faster where the comparator did lookups or copies.
Moves the sorts in query planning, the PerfettoSQL parser, SQL rewrites,
trace type detection, system probes, perf feature sections, CPU per UID,
the ninja log, the symbolizer, metatrace args, SurfaceFlinger layers,
interval intersect's operand order, JSON export, Android logs and the
memory graph over. This saves 110KB of trace_processor_shell's code on
Linux and 147KB of the Wasm module.
8b5cb9a to
a6e65a7
Compare
|
FYI I reworked thi a bit since your last look: instead of calling the comparator through a function pointer, callers now pass an integer key and there's a single shared I tested every callsite against its original sort: most are the same or faster, and the worst case is ~1.7ms extra per 100k Android log lines, which I think is fine. |
Vibhu-Agarwal
left a comment
There was a problem hiding this comment.
The by-key version is both simpler to read and better on every number in the description; nice
Summary
Adds
ColdSortByKeyandColdStableSortByKey, replacements forstd::sortandstd::stable_sortwhich sort by an integer key through one sort compiled once rather than once per call site, and moves the sorts which are not hot over to them. This saves 110KB oftrace_processor_shell's code on Linux and 147KB of the Wasm module.Why
std::sortandstd::stable_sortare templates: every element type and comparator gets its own copy, at 3-4KB each. Most of TP's sorts run once per query or once per trace on a handful of elements, where that code buys nothing.Changes
util/cold_sort.h: each caller extracts an integer key per element into{key, index}pairs, which onestd::sortcompiled incold_sort.ccorders, then moves the elements into place by following the permutation's cycles. Ties break on the index, so the sort is stable;ColdSortByKeyandColdStableSortByKeyshare it, and the names record whether a caller relies on that....Descendingvariants sort the other way. Up to 64 elements the pairs live on the stack, so small sorts never allocate.(z, id)is packed into one 64-bit key.std::sort: the executor and interpreter, the dataframe sort, the trace sorter, interval intersect's interval sort, the flamegraph, slice layout and heap graphs.Example
Benchmarks
Size, release builds:
trace_processor_shell.text, Linux x86-64trace_processor.wasmPer call site, one
{int64, uint32}sort by a lambda at-O2on x86-64:std::sortstd::stable_sortColdSortByKeySpeed on Linux x86-64, sorting by an
int64field:std::sort/ by keystd::sort/ by keyMocks of each moved call site, with its element type, key and comparator, show the sorts whose comparators did lookups or copies getting faster: JSON export 6-9x, CPU infos 7-20x, SurfaceFlinger 2.6-4x, trace type detection and the symbolizer 2.3x. The largest slowdown is Android logs, 1.7ms per 100k lines.
Testing
New
ColdSortTests check againststd::stable_sortacross sizes, including both sides of the stack storage threshold, and descending order, signed key extremes and move-only elements. Unit tests of the moved call sites and the full diff test suite pass.