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

Improve memory layout of locations, functions, and mappings #814

Merged
merged 3 commits into from
Jul 11, 2023

Conversation

kolesnikovae
Copy link
Contributor

@kolesnikovae kolesnikovae commented Jul 6, 2023

Addresses #797

The numbers below is an estimate of the memory consumption reduce:

pyroscope_head_size_bytes{type="functions"} 240 -> 72
pyroscope_head_size_bytes{type="locations"} 344 -> 152
pyroscope_head_size_bytes{type="mappings"}  192 -> 96

It is expected that the overall memory consumption of the cluster will be reduced by appx. 20%.

// case the line information above represents one of the multiple
// symbols. This field must be recomputed when the symbolization state of the
// profile changes.
IsFolded bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

struct padding should be improved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two changes comparing to the proto definition:

  1. MappingId type uint64 -> uint32;
  2. IsFolded moved next to MappingId, to avoid extra-padding.

The current type size is 48 bytes, given the 8-bytes alignment. Further improvement seems impossible.

@kolesnikovae kolesnikovae force-pushed the perf/locations-memory-representation branch from 7a15b6f to 45ff18a Compare July 10, 2023 08:07
@kolesnikovae kolesnikovae force-pushed the perf/locations-memory-representation branch from 45ff18a to 9e5565e Compare July 10, 2023 08:08
@kolesnikovae kolesnikovae marked this pull request as ready for review July 10, 2023 09:16
Copy link
Collaborator

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work, this will help our memory headroom 🙂

@kolesnikovae kolesnikovae merged commit 7f69298 into main Jul 11, 2023
17 checks passed
@kolesnikovae kolesnikovae deleted the perf/locations-memory-representation branch July 11, 2023 03:00
@kolesnikovae kolesnikovae self-assigned this Jul 13, 2023
simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jul 18, 2023
…phlare#814)

* Use separate representation for in-memory and parquet

* Ensure backward compatibility

* Remove unused function
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants