Skip to content
This repository was archived by the owner on Jul 22, 2020. It is now read-only.

Comments

Speed up alert fingerprint generation#129

Merged
prymitive merged 1 commit intomasterfrom
speedup-fingerprints
Jul 6, 2017
Merged

Speed up alert fingerprint generation#129
prymitive merged 1 commit intomasterfrom
speedup-fingerprints

Conversation

@prymitive
Copy link
Contributor

Dynamic fingerprints made the code much slower, pprof shows they are responsible for ~70% of all cpu usage for any API call. To make it worse they are applied to all alerts, since dedup layer doesn't know which alerts will be filtered later, it operates on all of them. This PR will:

  1. add benchmarks to so it's easier to track performance
  2. Keep current methods for accessing fingerprints, but use precomputed static fields in those
  3. Refactor Alert methods to use pointers, so we're not working on a copy

Benchmark timing went down from ~4000ns to 0.4ns for fingerprint calls and API response times from 1.3s (for my test sample) to 0.2s, which puts it back to the same level as before moving fingerprints to be dynamic. I still don't like this code much, it's all over the place, but I don't have a good idea how to better structure this, let's hope I'll be wiser in the future.

Dynamic fingerprints made the code much slower, pprof shows they are responsible for ~70% of all cpu usage for any API call. To make it worse they are applied to all alerts, since dedup layer doesn't know which alerts will be filtered later, it operates on all of them. This PR will:
1. add benchmarks to so it's easier to track performance
2. Keep current methods for accessing fingerprints, but use precomputed static fields in those
3. Refactor Alert methods to use pointers, so we're not working on a copy

Benchmark timing went down from ~4000ns to 0.4ns for fingerprint calls and API response times from 1.3s (for my test sample) to 0.2s, which puts it back to the same level as before moving fingerprints to be dynamic. I still don't like this code much, it's all over the place, but I don't have a good idea how to better structure this, let's hope I'll be wiser in the future.
@prymitive prymitive added the bug label Jul 6, 2017
@prymitive prymitive requested review from Tenzer and jamesog July 6, 2017 06:35
@prymitive prymitive merged commit 4fe7835 into master Jul 6, 2017
@prymitive prymitive deleted the speedup-fingerprints branch July 6, 2017 15:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants