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

pkg/profile: Replace gzip package to reduce allocs #1065

Merged
merged 1 commit into from
Nov 24, 2022

Conversation

marselester
Copy link
Contributor

A drop-in replacement for gzip has impressively low allocs with decent speed/compression klauspost/compress#216. See, gzip related code no longer shows up in the pprof top20.

(pprof) top20 -cum
Showing nodes accounting for 104.70MB, 40.74% of 257.02MB total
Dropped 109 nodes (cum <= 1.29MB)
Showing top 20 nodes out of 125
      flat  flat%   sum%        cum   cum%
         0     0%     0%   100.85MB 39.24%  github.com/oklog/run.(*Group).Run.func1
         0     0%     0%   100.85MB 39.24%  github.com/parca-dev/parca-agent/pkg/profiler/cpu.(*CPU).Run
         0     0%     0%   100.85MB 39.24%  main.run.func6
         0     0%     0%   100.85MB 39.24%  main.run.func6.1
         0     0%     0%   100.85MB 39.24%  runtime/pprof.Do
         0     0%     0%    73.71MB 28.68%  github.com/godbus/dbus/v5.(*Conn).inWorker
    8.58MB  3.34%  3.34%    73.71MB 28.68%  github.com/godbus/dbus/v5.(*unixTransport).ReadMessage
    9.11MB  3.55%  6.88%    64.13MB 24.95%  github.com/godbus/dbus/v5.DecodeMessage
         0     0%  6.88%    55.02MB 21.41%  github.com/godbus/dbus/v5.(*decoder).Decode
      50MB 19.46% 26.34%    55.02MB 21.41%  github.com/godbus/dbus/v5.(*decoder).decode
    3.02MB  1.17% 27.51%    54.02MB 21.02%  github.com/coreos/go-systemd/v22/dbus.(*Conn).SubscribeUnitsCustom.func1
         0     0% 27.51%       51MB 19.84%  github.com/coreos/go-systemd/v22/dbus.(*Conn).ListUnits (inline)
         0     0% 27.51%       51MB 19.84%  github.com/coreos/go-systemd/v22/dbus.(*Conn).ListUnitsContext
   12.49MB  4.86% 32.37%       50MB 19.45%  github.com/coreos/go-systemd/v22/dbus.(*Conn).listUnitsInternal
         0     0% 32.37%    44.89MB 17.47%  github.com/parca-dev/parca-agent/pkg/metadata.(*StatelessProvider).Labels
         0     0% 32.37%    44.89MB 17.47%  github.com/parca-dev/parca-agent/pkg/metadata/labels.(*Manager).LabelSet
         0     0% 32.37%    44.89MB 17.47%  github.com/parca-dev/parca-agent/pkg/metadata/labels.(*Manager).labelSet
         0     0% 32.37%    44.39MB 17.27%  github.com/parca-dev/parca-agent/pkg/metadata.Compiler.func1
         0     0% 32.37%    42.50MB 16.54%  debug/elf.Open
   21.50MB  8.37% 40.74%    41.50MB 16.15%  debug/elf.NewFile

ROUTINE ======================== github.com/parca-dev/parca-agent/pkg/profiler.(*RemoteProfileWriter).Write in pkg/profiler/profile_writer.go
         0        2MB (flat, cum)  0.78% of Total
         .          .     73:	buf := bytes.NewBuffer(nil)
         .          .     74:	zw, err := gzip.NewWriterLevel(buf, gzip.StatelessCompression)
         .          .     75:	if err != nil {
         .          .     76:		return err
         .          .     77:	}
         .        1MB     78:	if err = prof.WriteUncompressed(zw); err != nil {
         .          .     79:		zw.Close()
         .          .     80:		return err
         .          .     81:	}
         .          .     82:	zw.Close()
         .          .     83:
         .          .     84:	_, err = rw.profileStoreClient.WriteRaw(ctx, &profilestorepb.WriteRawRequest{
         .          .     85:		Normalized: true,
         .          .     86:		Series: []*profilestorepb.RawProfileSeries{{
         .        1MB     87:			Labels: &profilestorepb.LabelSet{Labels: convertLabels(labels)},
         .          .     88:			Samples: []*profilestorepb.RawSample{{
         .          .     89:				RawProfile: buf.Bytes(),
         .          .     90:			}},
         .          .     91:		}},
         .          .     92:	})

The discussion is here #1056.

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 7b667f9 into parca-dev:main Nov 24, 2022
@brancz
Copy link
Member

brancz commented Nov 24, 2022

Just deployed this on the demo cluster, and I'm happy to report that this got us a healthy ~20% decrease in CPU usage, nice job, and thank you very much! 🎉

@brancz
Copy link
Member

brancz commented Nov 24, 2022

I'm especially impressed that this didn't need any pooling of byte buffers, just the replacement 🤯

@marselester
Copy link
Contributor Author

Awesome! I am glad the change made a positive impact.

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.

3 participants