Skip to content

stats: use flat_hash_map and make thread-local stat maps use key storage from stat#4872

Merged
mattklein123 merged 59 commits intoenvoyproxy:masterfrom
jmarantz:flat-no-dup
Oct 29, 2018
Merged

stats: use flat_hash_map and make thread-local stat maps use key storage from stat#4872
mattklein123 merged 59 commits intoenvoyproxy:masterfrom
jmarantz:flat-no-dup

Conversation

@jmarantz
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz commented Oct 26, 2018

Description: A significant amount of storage is wasted in the maps in ThreadLocalStore with copies of the strings. This changes the maps to use const char* keys, which are held in every form of stat. This PR also switches the maps in thread-local-store to use flat_hash_map, which is particularly effective when the keys are just pointers, #4711 and #4715 are much clearer wins when submitted together, at the cost of complexity of review. This is one step toward resolving #4196.

It reduces memory from 73k/cluster to 56k/cluster in the Istio test referenced in #4196. In the memory-tests in thread_local_store_test.cc, it reduces the no-tls test from 31.5M to 21.2M, and the TLS memory test from 40.4M to 24.5M.

This change also makes things much faster:

current:

--------------------------------------------------------------
Benchmark                       Time           CPU Iterations
--------------------------------------------------------------
BM_StatsNoTls_mean       16249144 ns   16248983 ns         37
BM_StatsNoTls_median     16176011 ns   16175443 ns         37
BM_StatsNoTls_stddev       484905 ns     484909 ns         37
BM_StatsWithTls_mean     20958468 ns   20958244 ns         30
BM_StatsWithTls_median   20899829 ns   20899217 ns         30
BM_StatsWithTls_stddev     576702 ns     576784 ns         30

new (this PR):

--------------------------------------------------------------
Benchmark                       Time           CPU Iterations
--------------------------------------------------------------
BM_StatsNoTls_mean       13779759 ns   13779854 ns         39
BM_StatsNoTls_median     13630165 ns   13630325 ns         39
BM_StatsNoTls_stddev       576447 ns     576471 ns         39
BM_StatsWithTls_mean     16336826 ns   16336967 ns         35
BM_StatsWithTls_median   16240568 ns   16240563 ns         35
BM_StatsWithTls_stddev     544943 ns     544942 ns         35

A significant follow-up to this would be to tackle the redundant storage for tagExtractedName and TagVector. Another significant follow-up is to use symbolized strings (see #4696 )

Risk Level: low
Testing: //test/...
Docs Changes: n/a
Release Notes:n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>
… as a key.

Also reduce HeapStatAllocator key overhead by 8 bytes by just using the char*.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…ction boundaries around HeapStatData construction/destruction.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…mbol-table mem.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…d use it where helpful.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…tats are robust at runtime.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…tructure.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…cal_store.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…ass method comments.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz changed the title WIP stats: use flat_hash_map and make thread-local stat maps use key storage from stat stats: use flat_hash_map and make thread-local stat maps use key storage from stat Oct 26, 2018
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@mattklein123 mattklein123 self-assigned this Oct 27, 2018
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Very cool. LGTM, small comment.

Comment thread bazel/envoy_build_system.bzl Outdated
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@mattklein123 mattklein123 merged commit 22d18a4 into envoyproxy:master Oct 29, 2018
@jmarantz jmarantz deleted the flat-no-dup branch October 29, 2018 21:52
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.

2 participants