Skip to content

Performance improvements#9

Open
aboroska wants to merge 5 commits into
masterfrom
fix-insert-timestamp-purge-size
Open

Performance improvements#9
aboroska wants to merge 5 commits into
masterfrom
fix-insert-timestamp-purge-size

Conversation

@aboroska

@aboroska aboroska commented Feb 3, 2022

Copy link
Copy Markdown
  • Remove dead code

  • Reduce number of timestamp calls

    The default value in dict:update/4 is not wrapped in a fun and evaluated
    each time. That caused double timestamp calls when a metric was already
    added. To make things worse this was called for each inserted metric
    in every second.

  • Reduce memory usage of aggregator

    As the window is not changing runtime, no need to keep the last 5
    minutes of metrics.

    Keeping 300 elements in a list for each metric puts extra burden on the
    lists and dict functions in the statman_aggregator module.

    A 30 seconds window now makes the aggregator to keep 30 samples only.

@aboroska aboroska requested a review from a team February 3, 2022 17:10

@legoscia legoscia left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like Erlang 21 has bitrotted in Travis. We should probably replace it with some newer versions in .travis.yml.

The default value in dict:update/4 is not wrapped in a fun and evaluated
each time. That caused double timestamp calls when a metric was already
added. To make things worse this was called for each inserted metric
in every second.
As the window is not changing runtime, no need to keep the last 5
minutes of metrics.

Keeping 300 elements in a list for each metric puts extra burden on the
lists and dict functions in the statman_aggregator module.

A 30 seconds window now makes the aggregator to keep 30 samples only.
@aboroska aboroska force-pushed the fix-insert-timestamp-purge-size branch from 141d77f to a0a6ae7 Compare February 3, 2022 18:40
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