Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9732 )
Change subject: striped64: reduce size from 192 to 16 bytes ...................................................................... striped64: reduce size from 192 to 16 bytes Striped64 is the type backing counter metrics, and on a server with lots of tablets we have lots of counters. On a server with ~6400 tablets, I see about 50MB of memory used by these counters. I looked at it a few minutes and found that it could be made much smaller with a few small changes: - previously it used a virtual destructor, which required a vtbl to be stored. This is a best practice but in this case we always delete particular subclasses and don't hold on to references using the superclass, so I think it's worth bending the rules a bit. - each Striped64 stored 'num_cells_' which was always either 1 or the number of CPUs. Instead of storing this int, we can just use the nullness of the 'cells_' pointer to know whether it's set, and assume that when it's set, it always has a count equal to the number of CPUs. - previously it kept a void* cell_buffer_ which was identical to the Cell* cells_. That didn't make much sense, so this just does away with the void* version. - previously we used the 'Cell' class as the base counter value. 'Cell' is cacheline-padded to reduce false sharing. However, we assume that any high-throughput counter will expend out to its multiple-cell representation pretty quickly, so worrying about false sharing on low-throughput counters doesn't seem worth it. This patch replaces it with a simple atomic. - previously we used a 'busy_' boolean as a lock. This patch integrates that lock into the value of the cells_ pointer by using a special invalid pointer value -1 to indicate locked status. Overall this isn't a huge win - maybe 40-50M on a server with a huge number of tablets, but seemed like low hanging fruit, so why not? Change-Id: Id9ba955357744935ac6242cd3b609412c7e9da66 Reviewed-on: http://gerrit.cloudera.org:8080/9732 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Kudu Jenkins --- M src/kudu/util/striped64-test.cc M src/kudu/util/striped64.cc M src/kudu/util/striped64.h 3 files changed, 92 insertions(+), 87 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9732 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id9ba955357744935ac6242cd3b609412c7e9da66 Gerrit-Change-Number: 9732 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]>
