-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Closed
Labels
A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and toolsC-bugCategory: This is a bug.Category: This is a bug.T-libsRelevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.
Description
Location
rust/library/alloc/src/collections/btree/map.rs
Lines 48 to 49 in f6cb952
/// is stored in its own individually heap-allocated node. This means that every single insertion | |
/// triggers a heap-allocation, and every single comparison should be a cache-miss. Since these |
Summary
In the discussion of performance issues of binary search trees, it is written that [...] every single comparison should be a cache-miss.
I think this is confusing in the following ways:
- Comparisons are not cache misses. They operate on values which are already loaded into registers. Of course, the nodes need to be loaded in order to perform the comparisons, but that is not what is written in the relevant sentence.
- The use of
should
could be understood as a value judgement. I doubt it was intended as one, but rather to express that a cache miss is expected. However, even this interpretations is not fully accurate, since the next node for the tree traversal might be cached (because it was recently accessed or close enough in memory to something recently accessed that it was cached alongside).
I suggest replacing the last two sentences of the paragraph with something like This means that every single insertion triggers a heap-allocation, and for every comparison a node needs to be loaded, which could result in a cache miss. Since both heap-allocations and cache-misses are notably expensive in practise, we are forced to, at the very least, reconsider the BST strategy.
Metadata
Metadata
Assignees
Labels
A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and toolsC-bugCategory: This is a bug.Category: This is a bug.T-libsRelevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.