Functional simplification in the transposition table #5263
Conversation
bench: 1426724
| @@ -123,7 +123,7 @@ TTEntry* TranspositionTable::probe(const Key key, bool& found) const { | |||
| const uint16_t key16 = uint16_t(key); // Use the low 16 bits as key inside the cluster | |||
|
|
|||
| for (int i = 0; i < ClusterSize; ++i) | |||
| if (tte[i].key16 == key16 || !tte[i].depth8) | |||
| if (tte[i].key16 == key16) | |||
| return found = bool(tte[i].depth8), &tte[i]; | |||
There was a problem hiding this comment.
can you example now what the bool(tte[i].depth8) does, what's the typical value?
There was a problem hiding this comment.
can you example now what the bool(tte[i].depth8) does, what's the typical value?
bool(tte[i].depth8) checks for if the entry is found or not
the trick of the depth offsetting with -7 , is it makes depth 0 truthy from qsearch because int(0) is false..
Now since we use a pointer to the entry we can return the entry but not consider it as found, so I assume the older code, was trying to make the code smarter by forcing where to save the entry.
There was a problem hiding this comment.
Patch makes no sense, imho. As soon as we find an empty slot, we want to return immediately.
There was a problem hiding this comment.
but my question is, doesn't the patch already make an entire cluster non-functioning?
to me it seems like the first cluster is never used for saving here. because the loop to follow bellow starts from 1?
There was a problem hiding this comment.
It works because the first cluser is set as default replacement in line 130. The loop checks only if one of the other clusters in better.
There was a problem hiding this comment.
i.e. the entries to return from for a no-found entry, is always coming from replacement strategy below which is never from tte[0], this sounds fine, but at the same time, how can the first cluster ever have entries saved onto it, if we never say the first entry has empty slots and the first cluster never having any keys?
There was a problem hiding this comment.
Oh sorry indeed that works...
There was a problem hiding this comment.
Yes so what this patch does is it replaces always based on relative age and never on the bases that an entry is empty..
fascinating to me that it passed
|
I think this is very sensitive topic, logically I can't pass beyond how replacing old entries should be any better than filling out empty entries. I have concerns if this can scale with lots of memory available while we overwrite old entries for no reason. |
|
I think the reason this passes is because there are few if any empty entries left after just one or two moves of a game. Also the empty entries will start with generation 0 which will be older than any stored entries. IF we consider committing this I would suggest at least running some high hash pressure tests. Say 2MB STC or 8MB LTC. |
Can pls someone run them on my behalf? |
|
I'm happy to do it. Let's see what if anything the maintainers want first. |
|
I'm a bit skeptical about this patch, but would like to have the data of the patch at high hash pressure indeed. |
|
https://tests.stockfishchess.org/tests/view/66491b2fb8fa20e74c39f41d |
|
I would have thought that the test needed is less pressure rather than high pressure i.e. trying to show that filling empty entries is better than replacing old ones. |
|
You are probably right and this affects the low pressure scenario more than the high. The replacement policy should still select the empty entries first for replacement since they will be the oldest with the lowest depths. It's just that instead of returning an empty entry immediately we spend time executing the replacement policy code. On the other hand when the TT fills up and there are no empty entries left the |
(and fix a stray comment) I say "fix" due official-stockfish#5263 and its lengthy discussion. Beyond that, this is a clear readability and maintainbility improvement for beginners and experts alike. Passed STC non-reg: https://tests.stockfishchess.org/tests/view/66695b6a602682471b064cfc LLR: 2.93 (-2.94,2.94) <-1.75,0.25> Total: 107520 W: 28138 L: 27998 D: 51384 Ptnml(0-2): 373, 12257, 28358, 12401, 371 no functional change
Functional simplification in the transposition table.
Passed STC:
LLR: 2.98 (-2.94,2.94) <-1.75,0.25>
Total: 154848 W: 39838 L: 39750 D: 75260
Ptnml(0-2): 404, 16214, 44087, 16328, 391
https://tests.stockfishchess.org/tests/view/664892b088b8c6a2bbe430fc
Passed LTC:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 68172 W: 17296 L: 17137 D: 33739
Ptnml(0-2): 23, 6349, 21185, 6504, 25
https://tests.stockfishchess.org/tests/view/6648aabfa0781149e383e526
bench: 1426724