Skip to content
This repository was archived by the owner on May 23, 2023. It is now read-only.

Memory usage optimisations #5

Merged
merged 2 commits into from
Nov 3, 2022
Merged

Memory usage optimisations #5

merged 2 commits into from
Nov 3, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Oct 26, 2022

The changes improved benchmarks results across number of executions (+60%), time (-4300000 ns/op), memory (-23200000 B/op) and allocations (-800 allocs/op).

Parse changes Benchmark

name       old time/op    new time/op    delta
Parser-16    11.9ms ± 1%     7.4ms ± 1%  -37.77%  (p=0.008 n=5+5)

name       old alloc/op   new alloc/op   delta
Parser-16    30.7MB ± 0%     4.8MB ± 2%  -84.42%  (p=0.008 n=5+5)

name       old allocs/op  new allocs/op  delta
Parser-16     4.19k ± 0%     3.48k ± 0%  -16.84%  (p=0.008 n=5+5)

Comparing the memory profile of both executions, shows that the gains are primarily in the zlib writing code.
image

Reference.String() changes Benchmark

For bench mark delta on the ReferenceName optimisations:

name                        old time/op    new time/op    delta
ReferenceStringSymbolic-16     140ns ± 4%      40ns ± 9%  -71.19%  (p=0.008 n=5+5)
ReferenceStringHash-16         174ns ±14%      85ns ± 4%  -51.13%  (p=0.008 n=5+5)
ReferenceStringInvalid-16     48.9ns ± 2%     1.5ns ± 3%  -96.96%  (p=0.008 n=5+5)

name                        old alloc/op   new alloc/op   delta
ReferenceStringSymbolic-16     88.0B ± 0%     32.0B ± 0%  -63.64%  (p=0.008 n=5+5)
ReferenceStringHash-16          176B ± 0%      144B ± 0%  -18.18%  (p=0.008 n=5+5)
ReferenceStringInvalid-16      0.00B          0.00B          ~     (all equal)

name                        old allocs/op  new allocs/op  delta
ReferenceStringSymbolic-16      4.00 ± 0%      1.00 ± 0%  -75.00%  (p=0.008 n=5+5)
ReferenceStringHash-16          5.00 ± 0%      3.00 ± 0%  -40.00%  (p=0.008 n=5+5)
ReferenceStringInvalid-16       0.00           0.00          ~     (all equal)

Comparing source-controller execution without the changes (left) versus with the changes (right), shows that go_memstats_alloc_bytes_total seems to be able to release more of the allocated bytes:
image

The setup and configuration are the same as used for testing #4.

@pjbgf pjbgf added this to the GA milestone Oct 26, 2022
@pjbgf pjbgf added the area/git label Nov 1, 2022
@pjbgf pjbgf modified the milestones: GA, Bootstrap GA Nov 2, 2022
@hiddeco hiddeco self-requested a review November 2, 2022 10:41
@pjbgf pjbgf requested a review from aryan9600 November 3, 2022 08:27
@pjbgf pjbgf self-assigned this Nov 3, 2022
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
@pjbgf pjbgf force-pushed the memory-usage branch 2 times, most recently from 707e36e to 1253278 Compare November 3, 2022 13:52
Decreases allocations and bytes per operation by using string builder
with a predefined size.

One additional allocation has been removed by using its own implementation
of Strings(). The reason behind this was due to the fact the calls to
.String() are more recurrent than .Strings() and the performance impact
was worth the code duplication.

Benchmark results:
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
name                        old time/op    new time/op    delta
ReferenceStringSymbolic-16     140ns ± 4%      40ns ± 9%  -71.19%  (p=0.008 n=5+5)
ReferenceStringHash-16         174ns ±14%      85ns ± 4%  -51.13%  (p=0.008 n=5+5)
ReferenceStringInvalid-16     48.9ns ± 2%     1.5ns ± 3%  -96.96%  (p=0.008 n=5+5)

name                        old alloc/op   new alloc/op   delta
ReferenceStringSymbolic-16     88.0B ± 0%     32.0B ± 0%  -63.64%  (p=0.008 n=5+5)
ReferenceStringHash-16          176B ± 0%      144B ± 0%  -18.18%  (p=0.008 n=5+5)
ReferenceStringInvalid-16      0.00B          0.00B          ~     (all equal)

name                        old allocs/op  new allocs/op  delta
ReferenceStringSymbolic-16      4.00 ± 0%      1.00 ± 0%  -75.00%  (p=0.008 n=5+5)
ReferenceStringHash-16          5.00 ± 0%      3.00 ± 0%  -40.00%  (p=0.008 n=5+5)
ReferenceStringInvalid-16       0.00           0.00          ~     (all equal)

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Thanks a lot @pjbgf 🙇

@pjbgf pjbgf merged commit 1df4136 into master Nov 3, 2022
@pjbgf pjbgf deleted the memory-usage branch November 3, 2022 14:54
pjbgf added a commit that referenced this pull request Nov 4, 2022
Expands on the optimisations from #5
and ensures that zlib reader does not need to recreate a deflate
dictionary at every use.

The use of sync pools was consolidated into a new sync utils package.

name       old time/op    new time/op    delta
Parser-16    7.51ms ± 3%    7.71ms ± 6%     ~     (p=0.222 n=5+5)

name       old alloc/op   new alloc/op   delta
Parser-16    4.65MB ± 3%    1.90MB ± 3%  -59.06%  (p=0.008 n=5+5)

name       old allocs/op  new allocs/op  delta
Parser-16     3.48k ± 0%     3.32k ± 0%   -4.57%  (p=0.016 n=5+4)

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
pjbgf added a commit to pjbgf/go-git that referenced this pull request Nov 7, 2022
Expands on the optimisations from fluxcd#5
and ensures that zlib reader does not need to recreate a deflate
dictionary at every use.

The use of sync pools was consolidated into a new sync utils package.

name       old time/op    new time/op    delta
Parser-16    7.51ms ± 3%    7.71ms ± 6%     ~     (p=0.222 n=5+5)

name       old alloc/op   new alloc/op   delta
Parser-16    4.65MB ± 3%    1.90MB ± 3%  -59.06%  (p=0.008 n=5+5)

name       old allocs/op  new allocs/op  delta
Parser-16     3.48k ± 0%     3.32k ± 0%   -4.57%  (p=0.016 n=5+4)

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
gibchikafa pushed a commit to gibchikafa/go-git that referenced this pull request Nov 23, 2022
Expands on the optimisations from fluxcd#5
and ensures that zlib reader does not need to recreate a deflate
dictionary at every use.

The use of sync pools was consolidated into a new sync utils package.

name       old time/op    new time/op    delta
Parser-16    7.51ms ± 3%    7.71ms ± 6%     ~     (p=0.222 n=5+5)

name       old alloc/op   new alloc/op   delta
Parser-16    4.65MB ± 3%    1.90MB ± 3%  -59.06%  (p=0.008 n=5+5)

name       old allocs/op  new allocs/op  delta
Parser-16     3.48k ± 0%     3.32k ± 0%   -4.57%  (p=0.016 n=5+4)

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants