Undo refactoring in XOR encoding#11472
Conversation
PromQL benchmark comparing `sparsehistogram` branch and this PR |
d089948 to
b21011d
Compare
|
I basically copied the xor.go from main branch and moved xorWrite and xorRead to histogram.go file. |
b21011d to
62aed48
Compare
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
62aed48 to
ad641e3
Compare
|
Hmm, this doesn't feel good. So much complex code replicated… I haven't quite come to terms with this. Here just a few brainstormed questions that go through my head:
|
|
I should definitely dig a bit deeper; I didn't look at the profiles for this. I will convert this PR to draft for now. The bigger issue is some of the queries in this result that are significantly slower and not related to this PR, and that is where I plan to spend some more time before I get this this PR again. |
|
I investigated this a bit. First I tried to make the call cheaper by passing in pointers to the leading/trailing zero bits, and only update them if needed (assuming that not updating is the happy path and shouldn't inflict additional overhead). However, that didn't make a dent. Then I looked at Next, I used AFAIK we cannot force the Go compiler to inline more aggressively. So the only course of action here seems to be the manual inlining as proposed in this PR. However the question remains if any action is actually required. We should still find out how relevant the overhead is in practice. Once we add support for float histograms, we will use the code in |
|
Merged #11476, not sure how much it helps but looked fine. |
NOTE: sparsehistogram branch,
The XOR chunk refactoring made it ~2% slower to read. This PR takes it back.
This is a small step and does not fix the sloweness in some queries that we see with histogram code.
A comment with benchmark results will follow.