Introduce String Hash Map to speed up aggregation over short string keys. #6243

akuzm · 2019-07-31T15:47:08Z

This PR introduces the string hash map that speeds up the aggregation over short string keys, finishing the series of patches from #5417.

Category:

Performance Improvement

Changelog entry:
The performance of aggregation over short string keys is improved.

akuzm · 2019-10-17T14:56:09Z

No significant difference on this query:

SELECT URL, count() AS c FROM hits_100m_single GROUP BY URL ORDER BY c DESC LIMIT 10
┏━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━┳━━━━━━━┳━━━━━━━┳━━━━━━━━━┓
┃ version               ┃ count() ┃   min ┃   med ┃   max ┃ q95-q05 ┃
┡━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━╇━━━━━━━╇━━━━━━━╇━━━━━━━━━┩
│ /home/akuzm/ch-string │     161 │ 6.832 │ 7.107 │ 7.296 │   0.221 │
├───────────────────────┼─────────┼───────┼───────┼───────┼─────────┤
│ /home/akuzm/ch-master │     161 │ 6.935 │ 7.108 │  7.29 │    0.22 │
└───────────────────────┴─────────┴───────┴───────┴───────┴─────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ q                                             ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
│ [0,0.005000114440917969,0.018000125885009766] │
└───────────────────────────────────────────────┘
result-10.17-16.23.01.txt

akuzm · 2019-10-18T16:45:30Z

That's what I got from string_hash_map test, with ArenaKeyHolder, 49 runs:

┏━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━┳━━━━━━━━┳━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━┓
┃ file   ┃ version              ┃    min ┃    med ┃    max ┃ q95-q05 ┃        mem ┃
┡━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━╇━━━━━━━━╇━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━┩
│ term1  │ HashMapWithSavedHash │  1.061 │  1.075 │  1.117 │    0.04 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term1  │ StringHashMap        │  0.558 │  0.565 │  0.594 │   0.021 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term2  │ HashMapWithSavedHash │  2.181 │  2.213 │  2.303 │   0.054 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term2  │ StringHashMap        │  1.019 │  1.033 │  1.091 │    0.04 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term4  │ HashMapWithSavedHash │  5.229 │  5.276 │  5.558 │   0.112 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term4  │ StringHashMap        │  2.435 │  2.463 │   2.54 │    0.08 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term8  │ HashMapWithSavedHash │  8.192 │  8.284 │  9.519 │    0.88 │  402653184 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term8  │ StringHashMap        │  5.586 │  5.653 │  5.735 │   0.128 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term16 │ HashMapWithSavedHash │  8.348 │  8.441 │  8.788 │   0.311 │  805306368 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term16 │ StringHashMap        │  6.822 │  6.894 │   7.73 │   0.088 │  134217728 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term24 │ HashMapWithSavedHash │  8.754 │  8.883 │  9.245 │   0.297 │ 1207959552 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term24 │ StringHashMap        │  8.146 │  8.245 │  9.893 │   0.232 │  671088640 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term48 │ HashMapWithSavedHash │  9.601 │  9.679 │ 10.469 │   0.326 │ 2415919104 │
├────────┼──────────────────────┼────────┼────────┼────────┼─────────┼────────────┤
│ term48 │ StringHashMap        │ 10.198 │ 10.349 │ 11.339 │   0.785 │ 2415919104 │
└────────┴──────────────────────┴────────┴────────┴────────┴─────────┴────────────┘

All short strings improve, and for long strings there is 5% regression which shouldn't be noticeable in real workload. We'll see what the CI performance test says.

akuzm · 2019-10-18T16:49:11Z

The next thing I want to do is to study the coverage info -- e.g. mergeToViaFind is not covered at all.

akuzm · 2019-10-21T13:10:03Z

SELECT MobilePhoneModel, uniq(UserID) AS u FROM hits_100m_single WHERE MobilePhoneModel != '' GROUP BY MobilePhoneModel ORDER BY u DESC LIMIT 10
┏━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━┳━━━━━━━┳━━━━━━━┳━━━━━━━━━┓
┃ version               ┃ count() ┃   min ┃   med ┃   max ┃ q95-q05 ┃
┡━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━╇━━━━━━━╇━━━━━━━╇━━━━━━━━━┩
│ /home/akuzm/ch-string │     161 │ 0.657 │ 0.671 │ 0.747 │   0.037 │
├───────────────────────┼─────────┼───────┼───────┼───────┼─────────┤
│ /home/akuzm/ch-master │     161 │  0.66 │ 0.671 │ 0.726 │   0.038 │
└───────────────────────┴─────────┴───────┴───────┴───────┴─────────┘

This query regresses x1.23 in performance test, but I can't reproduce the difference. According to perf report, the time is dominated by reading the table, applying the prewhere clause and calculating uniq(UserID), that's why we are not seeing any changes.

It speeds up aggregation over short string keys. Use it as a default aggregation method for string keys.

amosbird · 2019-10-21T13:37:24Z

dbms/src/Common/HashTable/TwoLevelStringHashTable.h

+        switch (sz)
+        {
+            case 0:
+                keyHolderDiscardKey(key_holder);


hmm, we don't persist yet, why discard?

emplace() guarantees that it will either persist or discard the key. It doesn't pass the zero key_holder forward, and instead passes the empty key0, so it doesn't need the key from key_holder and should discard it here.

Actually I'm not sure when leaving it out would have any visible consequences -- discard is only defined for SerializedKeyHolder, and if it is zero, it's going to have zero size, and the discard would be a noop.

emplace() guarantees that it will either persist or discard the key.

Hmm, but dispatch is also used for find() and potentially hash().

For uses other than emplace, it should always be the NoopKeyHolder, for which this call does nothing. I noticed that the signature of FindCallable::operator () is somewhat misleading now, because it accepts a template KeyHolder. I'll change it to only accept the NoopKeyHolder.

Oops, we don't have the noop key holder anymore. It's represented by just the plain key now.

I just realized I've answered the question about OnExistingKey and OnNewKey to you #5417 (comment). Heh, now it confuses me. Those names do convey more than they should.

Well, I think the persist/discard is better that onNew/onExisting, but maybe still bad indeed. Don't know how to rename it. Basically, it is a way for a hash table to say that it either needs a persistent version of this key, or it doesn't. Something like makeKeyPersistent/dontMakeKeyPersistent? This also may be bad, because for SerializedKeyHolder, discardKey actually destroys it, and you can't use it anymore.

akuzm · 2019-10-21T14:50:50Z

The ubsan failure is in preciseExp10, not related to this PR.

akuzm · 2019-10-22T12:11:02Z

With the latest dispatch improvement (switch((sz - 1) >> 3) there is even less regression on string_hash_map test:

┏━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━┳━━━━━━━┳━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━┓
┃ file   ┃ version              ┃ count() ┃   min ┃   med ┃    max ┃ q95-q05 ┃        mem ┃
┡━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━╇━━━━━━━╇━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━┩
│ term1  │ HashMapWithSavedHash │      21 │ 1.058 │ 1.068 │    1.1 │   0.027 │  268435456 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term1  │ StringHashMap        │      21 │ 0.539 │ 0.545 │  0.565 │   0.019 │  167794720 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term2  │ HashMapWithSavedHash │      21 │ 2.197 │  2.21 │  2.285 │   0.068 │  671088640 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term2  │ StringHashMap        │      21 │ 1.057 │ 1.075 │  1.342 │   0.255 │  269238304 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term4  │ HashMapWithSavedHash │      21 │ 5.257 │ 5.303 │  5.413 │    0.13 │ 2281701376 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term4  │ StringHashMap        │      21 │  2.61 │ 2.627 │  2.672 │   0.053 │  721436704 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term8  │ HashMapWithSavedHash │      21 │ 8.185 │ 8.251 │  8.683 │   0.127 │ 4697620480 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term8  │ StringHashMap        │      21 │ 5.742 │ 5.778 │  5.907 │   0.141 │ 2826969120 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term16 │ HashMapWithSavedHash │      21 │ 8.254 │ 8.451 │  9.588 │   0.557 │ 5100273664 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term16 │ StringHashMap        │      21 │  6.95 │ 7.035 │  8.062 │   0.595 │ 3992977440 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term24 │ HashMapWithSavedHash │      21 │ 8.741 │ 8.836 │  9.085 │   0.305 │ 5502926848 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term24 │ StringHashMap        │      21 │ 8.182 │ 8.348 │   8.54 │   0.332 │ 5066784800 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term48 │ HashMapWithSavedHash │      21 │ 9.566 │ 9.695 │ 10.905 │   1.231 │ 6710886400 │
├────────┼──────────────────────┼─────────┼───────┼───────┼────────┼─────────┼────────────┤
│ term48 │ StringHashMap        │      21 │ 9.827 │ 9.902 │ 10.856 │   0.877 │ 6710904864 │
└────────┴──────────────────────┴─────────┴───────┴───────┴────────┴─────────┴────────────┘

akuzm · 2019-10-22T17:00:11Z

Functional stateless tests (debug) failure is out of disk space in the CI.

akuzm force-pushed the aku/hashtables branch from 8d517a9 to 7be15b9 Compare August 1, 2019 13:04

akuzm force-pushed the aku/hashtables branch from 7be15b9 to 16d9136 Compare August 9, 2019 12:18

akuzm changed the base branch from 19.11 to master August 9, 2019 12:28

akuzm force-pushed the aku/hashtables branch 5 times, most recently from 744e913 to 5f3057f Compare August 14, 2019 11:09

akuzm force-pushed the aku/hashtables branch 4 times, most recently from 79ce3e1 to 2bb68c2 Compare August 21, 2019 10:23

akuzm mentioned this pull request Aug 22, 2019

Add StringHashMap to optimize string aggregation #5417

Closed

akuzm force-pushed the aku/hashtables branch from 2bb68c2 to d3e09f9 Compare August 30, 2019 11:00

akuzm changed the title ~~[wip] memory management in hash tables~~ [wip] string hash tables review Aug 30, 2019

akuzm force-pushed the aku/hashtables branch from 16c9554 to c5cc239 Compare September 6, 2019 13:34

akuzm force-pushed the aku/hashtables branch 2 times, most recently from c00ddeb to ce3671e Compare September 25, 2019 11:47

akuzm mentioned this pull request Oct 17, 2019

StringHashMap revamped #7292

Closed

akuzm force-pushed the aku/hashtables branch from ce3671e to 59f92eb Compare October 17, 2019 10:21

akuzm force-pushed the aku/hashtables branch from 6232528 to 0736895 Compare October 21, 2019 13:17

github-actions bot added the pr-build Pull request with build/testing/packaging improvement label Oct 21, 2019

akuzm changed the title ~~[wip] string hash tables review~~ Introduce String Hash Map to speed up aggregation over short string keys. Oct 21, 2019

akuzm marked this pull request as ready for review October 21, 2019 13:26

Introduce String Hash Map.

a864447

It speeds up aggregation over short string keys. Use it as a default aggregation method for string keys.

akuzm force-pushed the aku/hashtables branch from 746f480 to a864447 Compare October 21, 2019 13:27

amosbird reviewed Oct 21, 2019

View reviewed changes

akuzm added pr-performance Pull request with some performance improvements and removed pr-build Pull request with build/testing/packaging improvement labels Oct 21, 2019

Review fixes.

d865aeb

github-actions bot added the pr-build Pull request with build/testing/packaging improvement label Oct 21, 2019

blinkov removed the pr-build Pull request with build/testing/packaging improvement label Oct 21, 2019

Port the latest dispatch() improvements.

dfd9f26

akuzm merged commit 1a609c2 into master Oct 23, 2019

akuzm deleted the aku/hashtables branch December 9, 2019 15:29

filimonov mentioned this pull request Jan 21, 2020

clickhouse crash " Received signal Segmentation fault" #8636

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce String Hash Map to speed up aggregation over short string keys. #6243

Introduce String Hash Map to speed up aggregation over short string keys. #6243

akuzm commented Jul 31, 2019 •

edited

akuzm commented Oct 17, 2019

akuzm commented Oct 18, 2019 •

edited

akuzm commented Oct 18, 2019

akuzm commented Oct 21, 2019

amosbird Oct 21, 2019

akuzm Oct 21, 2019

amosbird Oct 21, 2019

akuzm Oct 21, 2019

akuzm Oct 21, 2019

amosbird Oct 21, 2019 •

edited

akuzm Oct 21, 2019

akuzm commented Oct 21, 2019

akuzm commented Oct 22, 2019

akuzm commented Oct 22, 2019 •

edited

Introduce String Hash Map to speed up aggregation over short string keys. #6243

Introduce String Hash Map to speed up aggregation over short string keys. #6243

Conversation

akuzm commented Jul 31, 2019 • edited

akuzm commented Oct 17, 2019

akuzm commented Oct 18, 2019 • edited

akuzm commented Oct 18, 2019

akuzm commented Oct 21, 2019

amosbird Oct 21, 2019

Choose a reason for hiding this comment

akuzm Oct 21, 2019

Choose a reason for hiding this comment

amosbird Oct 21, 2019

Choose a reason for hiding this comment

akuzm Oct 21, 2019

Choose a reason for hiding this comment

akuzm Oct 21, 2019

Choose a reason for hiding this comment

amosbird Oct 21, 2019 • edited

Choose a reason for hiding this comment

akuzm Oct 21, 2019

Choose a reason for hiding this comment

akuzm commented Oct 21, 2019

akuzm commented Oct 22, 2019

akuzm commented Oct 22, 2019 • edited

akuzm commented Jul 31, 2019 •

edited

akuzm commented Oct 18, 2019 •

edited

amosbird Oct 21, 2019 •

edited

akuzm commented Oct 22, 2019 •

edited