i#7731: Optimize reuse distance collection using splay tree #7732
Conversation
artur-abd
changed the title
artur-abd
force-pushed
the
i7731-optimize-reuse-distance
branch
2 times, most recently
from
4b6bffb to
3961c95
Compare
Improve reuse distance using splay tree instead of linked list. Splay tree have asymptotic O(log n) amortizated time for all operations instead of O(n / const) for linked list with skip list. Perfomance improve achives in avarage 10-40% based on different benchmarks. Fixes DynamoRIO#7731
artur-abd
force-pushed
the
i7731-optimize-reuse-distance
branch
from
3961c95 to
1b8aa74
Compare
|
Thank you for contributing. Please mark for review when ready. Also, please don't force-push a shared branch: see https://dynamorio.org/page_code_reviews.html about squash-and-merge, where the final merge commit message comes from, and how force pushes break the review process. Use separate incremental commits with commit messages that describe what each one is doing (and not just repeating the merge message). |
Fixes description for line_ref_splay_t and it's methods Fixes: DynamoRIO#7731
|
Thanks for your comments. PR is ready for review. |
| // The head of the list is the most recently accessed cache line. | ||
| // The earlier a cache line was accessed last time, the deeper that cache line | ||
| // is in the list. | ||
| // We use a splay to keep track of the cache line reuse distance. |
There was a problem hiding this comment.
We use a splay to keep
splay tree
| // The earlier a cache line was accessed last time, the deeper that cache line | ||
| // is in the list. | ||
| // We use a splay to keep track of the cache line reuse distance. | ||
| // The lefmost node of the splay tree is the most recently accessed cache line. |
There was a problem hiding this comment.
The lefmost node of the splay tree is the most recently accessed cache line.
I thought splay trees have the most recently accessed node closer to the root, not to the left.
| : head_(NULL) | ||
| , gate_(NULL) | ||
| , tail_(NULL) | ||
| // The splay tree is a binary search tree that using splay operatio for balancing, it |
There was a problem hiding this comment.
The splay tree is a binary search tree that using splay operatio for balancing, it
allow to delete and insert an element at any position in O(log n) amortizated time.
Reword for grammar:
The splay tree is a binary search tree that uses the splay operation for balancing, which
allows deleting and inserting an element at any position in O(log n) amortized time.
| return root; | ||
| } | ||
|
|
||
| // Find previos element of ref in the splay tree. |
There was a problem hiding this comment.
previos
previous
and also in other comments.
| return nullptr; | ||
| // the last visited node | ||
| line_ref_node_t *last = nullptr; | ||
| // Walk up by tree wihle did not found a node to the left of the current |
There was a problem hiding this comment.
wihle did not found
while we do not find
| , tail_(NULL) | ||
| // The splay tree is a binary search tree that using splay operatio for balancing, it | ||
| // allow to delete and insert an element at any position in O(log n) amortizated time. | ||
| struct line_ref_splay_t { |
There was a problem hiding this comment.
Partially done with reviewing the splay tree functions here. A couple high level comments:
- I think it'll be useful to move this implementation to ext/drcontainers
- We also need a dedicated set of unit tests for the splay tree impl. It is complex enough to warrant separate test coverage.
3 participants
Improve reuse distance using splay tree instead of linked list. Splay tree have asymptotic O(log n) amortizated time for all operations instead of O(n / const) for linked list with skip list.
Perfomance improve achives in avarage 10-40% based on different benchmarks.
Fixes #7731