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

Dedupe merge improvement. #656

Merged
merged 18 commits into from
May 10, 2023
Merged

Dedupe merge improvement. #656

merged 18 commits into from
May 10, 2023

Conversation

cyriltovena
Copy link
Collaborator

@cyriltovena cyriltovena commented Apr 26, 2023

This PR improves the deduping of profiles using a loser tree implementation from @bboreham.

benchstat before.txt after.txt
name                       old time/op    new time/op    delta
SelectMergeStacktraces-16     54.8s ± 6%      2.5s ± 1%   -95.49%  (p=0.016 n=5+4)

name                       old alloc/op   new alloc/op   delta
SelectMergeStacktraces-16    6.92GB ± 0%    2.69GB ± 0%   -61.14%  (p=0.008 n=5+5)

name                       old allocs/op  new allocs/op  delta
SelectMergeStacktraces-16      133M ± 0%        0M ± 1%  -100.00%  (p=0.008 n=5+5)

A bit part of it is also a bug where we would create a goroutine for each profile inspected.

The PR also adds some more instrumentation and a benchmark for future improvements I'm planning to do on the merge of stracktraces.

Currently stacktraces are not merge in the benchmark but I plan to followup on this.

@cyriltovena cyriltovena marked this pull request as ready for review April 26, 2023 22:19
@cyriltovena cyriltovena merged commit 2e318c0 into main May 10, 2023
@cyriltovena cyriltovena deleted the querier-dedupe-concurency branch May 10, 2023 06:20
simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
* Avoid creating a new goroutine for every profiles when deduping

* Removes gzip compression when using querier

* Removes next async

* Only async for the first batch

* Do not sort in ingester and reduce batch size

* Reuse rewrite array

* Revert batch size

* Add a span

* Loser tree

* Correctly count dupe

* Removes allocations from At

* Remove allocation from the keep responses

* Removes gzip compression change

* Fetch the first batch async

* Removes sort change.

* make fmt lint
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants