Simple and flat ArrayTable
#18
Conversation
Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
|
|
kbieganski
changed the title
ArrayTable and ObjectTableArrayTable
|
I dropped the change to |
|
Do you have any benchmark measurements to show improvement? I think it is unlikely to be approved unless there are. |
|
The goal of this PR was to simplify the code, but I observed about 1% speedup on BlackParrot and 2% on Ibex. |
| @@ -657,7 +657,7 @@ Graph::makePrevPaths(Vertex *vertex, | |||
| } | |||
|
|
|||
| PathVertexRep * | |||
| Graph::prevPaths(Vertex *vertex) const | |||
| Graph::prevPaths(Vertex *vertex) | |||
There was a problem hiding this comment.
The loss of one indirection means we can't use const here, but if we're getting non-const pointers to vertices, then I would argue the graph is not actually const.
| void deleteBlocks(); | ||
|
|
||
| size_t size_; | ||
| // Block index of free block (blocks_[size - 1]). | ||
| BlockIdx free_block_idx_; | ||
| // Index of next free object in free_block_idx_. | ||
| ObjectIdx free_idx_; | ||
| // Don't use std::vector so growing blocks_ can be thread safe. |
There was a problem hiding this comment.
I guess you missed the comment that you deleted that explains why std::vector cannot be used here. If you look at the commit history you will see that it used to use std::vector. When the the vector grows it fails with multiple threads.
There was a problem hiding this comment.
I didn't miss it. vector::swap is basically no different from the previous approach. See https://godbolt.org/z/8sqqTPMMf
However, I suppose implementations other than libstdc++ might exhibit different behavior on lower optimization levels. So I pushed a manual swap implementation just to be sure.
Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
|
@jjcherry56 Do you have any further feedback? |
This patch simplifies the implementation of
ArrayTable.Instead of being an array of pointers to pointers, it is now flattened into an array of pointers.
Also, it now uses
std::vector, leading to less manual memory management and overall less manual bookkeeping.I expect this to bring a slight speedup, but I haven't measured it properly yet.