Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

receive: Improved Performance for err path. #4085

Merged
merged 1 commit into from
Apr 21, 2021
Merged

receive: Improved Performance for err path. #4085

merged 1 commit into from
Apr 21, 2021

Conversation

bwplotka
Copy link
Member

benchstat -delta-test=none ../_dev/thanos/2021/receive2/impr5-go1.16.3.txt ../_dev/thanos/2021/receive2/errimpr1-go1.16.3.txt
name                                                                                old time/op    new time/op    delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                        1.22ms ± 0%    1.05ms ± 0%  -13.97%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12           5.84ms ± 0%    1.26ms ± 0%  -78.46%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                       12.8ms ± 0%    11.3ms ± 0%  -11.54%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12          59.4ms ± 0%    12.8ms ± 0%  -78.39%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                  109ms ± 0%      97ms ± 0%  -11.16%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12     1.44s ± 0%     0.10s ± 0%  -93.25%

name                                                                                old alloc/op   new alloc/op   delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                        1.20MB ± 0%    1.20MB ± 0%   -0.69%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12           4.34MB ± 0%    1.36MB ± 0%  -68.54%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                       13.3MB ± 0%    13.2MB ± 0%   -0.67%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12          44.6MB ± 0%    14.9MB ± 0%  -66.67%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                  120MB ± 0%     120MB ± 0%   -0.00%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12     592MB ± 0%     120MB ± 0%  -79.77%

name                                                                                old allocs/op  new allocs/op  delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                         5.10k ± 0%     4.59k ± 0%   -9.83%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12            25.1k ± 0%      7.6k ± 0%  -69.67%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                        50.4k ± 0%     45.4k ± 0%   -9.93%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12            250k ± 0%       75k ± 0%  -69.96%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                   89.0 ± 0%      78.0 ± 0%  -12.36%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12       368 ± 0%       201 ± 0%  -45.38%

Signed-off-by: Bartlomiej Plotka bwplotka@gmail.com

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Verification

```
benchstat -delta-test=none ../_dev/thanos/2021/receive2/impr5-go1.16.3.txt ../_dev/thanos/2021/receive2/errimpr1-go1.16.3.txt
name                                                                                old time/op    new time/op    delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                        1.22ms ± 0%    1.05ms ± 0%  -13.97%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12           5.84ms ± 0%    1.26ms ± 0%  -78.46%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                       12.8ms ± 0%    11.3ms ± 0%  -11.54%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12          59.4ms ± 0%    12.8ms ± 0%  -78.39%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                  109ms ± 0%      97ms ± 0%  -11.16%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12     1.44s ± 0%     0.10s ± 0%  -93.25%

name                                                                                old alloc/op   new alloc/op   delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                        1.20MB ± 0%    1.20MB ± 0%   -0.69%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12           4.34MB ± 0%    1.36MB ± 0%  -68.54%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                       13.3MB ± 0%    13.2MB ± 0%   -0.67%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12          44.6MB ± 0%    14.9MB ± 0%  -66.67%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                  120MB ± 0%     120MB ± 0%   -0.00%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12     592MB ± 0%     120MB ± 0%  -79.77%

name                                                                                old allocs/op  new allocs/op  delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                         5.10k ± 0%     4.59k ± 0%   -9.83%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12            25.1k ± 0%      7.6k ± 0%  -69.67%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                        50.4k ± 0%     45.4k ± 0%   -9.93%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12            250k ± 0%       75k ± 0%  -69.96%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                   89.0 ± 0%      78.0 ± 0%  -12.36%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12       368 ± 0%       201 ± 0%  -45.38%
```

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
@bwplotka
Copy link
Member Author

With this, it's as minimal as it can be I think

@@ -1293,7 +1293,7 @@ func Heap(dir string) (err error) {
return err
}

f, err := os.Create(filepath.Join(dir, "impr5-go1.16.3.pprof"))
f, err := os.Create(filepath.Join(dir, "errimpr1-go1.16.3.pprof"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it only for testing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I provably should remove those before merging but those are super useful for checking leaks (:

Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@kakkoyun kakkoyun merged commit ad8dc2a into main Apr 21, 2021
@kakkoyun kakkoyun deleted the receive-err branch April 21, 2021 06:55
Copy link
Member

@onprem onprem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see any code changes contributing to the benchmarks here 😅. Are we missing some changes?

Copy link
Member Author

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

clyang82 pushed a commit to stolostron/thanos that referenced this pull request Apr 27, 2021
```
benchstat -delta-test=none ../_dev/thanos/2021/receive2/impr5-go1.16.3.txt ../_dev/thanos/2021/receive2/errimpr1-go1.16.3.txt
name                                                                                old time/op    new time/op    delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                        1.22ms ± 0%    1.05ms ± 0%  -13.97%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12           5.84ms ± 0%    1.26ms ± 0%  -78.46%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                       12.8ms ± 0%    11.3ms ± 0%  -11.54%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12          59.4ms ± 0%    12.8ms ± 0%  -78.39%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                  109ms ± 0%      97ms ± 0%  -11.16%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12     1.44s ± 0%     0.10s ± 0%  -93.25%

name                                                                                old alloc/op   new alloc/op   delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                        1.20MB ± 0%    1.20MB ± 0%   -0.69%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12           4.34MB ± 0%    1.36MB ± 0%  -68.54%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                       13.3MB ± 0%    13.2MB ± 0%   -0.67%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12          44.6MB ± 0%    14.9MB ± 0%  -66.67%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                  120MB ± 0%     120MB ± 0%   -0.00%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12     592MB ± 0%     120MB ± 0%  -79.77%

name                                                                                old allocs/op  new allocs/op  delta
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12                         5.10k ± 0%     4.59k ± 0%   -9.83%
HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12            25.1k ± 0%      7.6k ± 0%  -69.67%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                        50.4k ± 0%     45.4k ± 0%   -9.93%
HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12            250k ± 0%       75k ± 0%  -69.96%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                   89.0 ± 0%      78.0 ± 0%  -12.36%
HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12       368 ± 0%       201 ± 0%  -45.38%
```

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants