Merge query cache and query state#156632
Conversation
|
And there're the benchmark results for 4 threads on my MacBook Pro M1 Pro:
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This comment has been minimized.
This comment has been minimized.
8ec4ec7 to
54f6a8e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Can't get the parallel frontend cycle errors quite right as cycle breaking code cannot find any. This seems to only happen on a second break_query_cycle call tho. So I think maybe I didn't account for that somehow when thinking about order of operations or smth. |
|
Ah, and single-threaded cycle recovery haven't yet been put back too. |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Merge query cache and query state
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (0ac810c): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.8%, secondary 2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 5.6%, secondary 5.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 513.065s -> 530.367s (3.37%) |
fde8deb to
71e0c11
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #157149) made this pull request unmergeable. Please resolve the merge conflicts. |
So basically we have query cache and query state separate "hash" tables. Cache tracks completed queries while state tracks unfinished queries. Thus we are required to a double check lookup while updating one of them like in
try_execute_queryand we do multiple hash table lookups while executing a query since cache/state entry might have moved due to rehashing. This PR merges query cache with query state and stores query cache entries in an arena for its stable memory location.However my manually written slop code currently breaks query cycle detection, but I know how to fix it.More details in #t-compiler/query-system > Merging query cache and query state