thread-local arena: grow buf if there is no enough space #116
Conversation
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
skiplist/src/arena.rs
Outdated
| @@ -44,13 +45,13 @@ impl Arena { | |||
| pub fn alloc(&self, align: usize, size: usize) -> u32 { | |||
There was a problem hiding this comment.
Add a debug assertion align.is_power_of_two()
skiplist/src/arena.rs
Outdated
| // 0 means there is not enough space to allocate. | ||
| return 0; | ||
| } | ||
| offset as u32 |
There was a problem hiding this comment.
I thought the returning offset is supposed to be aligned.
There was a problem hiding this comment.
I'm considering what's the benefits of aligning to the number smaller than size of pointer. Should we simply announce that the align must >= sizeof(ptr), to make the code simple.
| let offset = arena.offset::<u64>(ptr); | ||
| assert_eq!(offset, 8); | ||
| } | ||
|
|
There was a problem hiding this comment.
Allocate a more aligned ptr on the same arena: alloc(16,16)
There was a problem hiding this comment.
Currently we simply alloc align to 8
There was a problem hiding this comment.
Actually some libraries and compilers expects to align with 16. Check jemalloc/jemalloc#1533. Perhaps we should add static assert on all types we want to store can be aligned with 8.
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
skiplist/src/arena.rs
Outdated
| @@ -30,7 +32,8 @@ impl Arena { | |||
| mem::forget(buf); | |||
| Arena { | |||
| core: Arc::new(ArenaCore { | |||
| len: AtomicU32::new(1), | |||
| // Reserve 8 bytes at the beginning | |||
There was a problem hiding this comment.
what's the purpose of reservation?
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
skiplist/src/arena.rs
Outdated
| @@ -30,7 +32,8 @@ impl Arena { | |||
| mem::forget(buf); | |||
| Arena { | |||
| core: Arc::new(ArenaCore { | |||
| len: AtomicU32::new(1), | |||
| // Reserve 8 bytes at the beginning, because func offset return 0 means invalid value | |||
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
|
Please fix the lint @zhangjinpeng1987 |
zhangjinpeng87
changed the title
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
zhangjinpeng87
changed the title
|
@Connor1996 PTAL again. I have updated this PR for more changes, please refer to the description. |
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
skiplist/src/arena.rs
Outdated
| pub struct Arena { | ||
| core: Arc<ArenaCore>, | ||
| core: ArenaCore, |
There was a problem hiding this comment.
As Arena is just a direct wrapper of AreanaCore, seems no need to introduce ArenaCore anymore
There was a problem hiding this comment.
Done @Connor1996 , PTAL again.
Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
Signed-off-by: zhangjinpeng1987 zhangjinpeng@pingcap.com
Changes