-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resize-in-place for large allocs. (Shrink-only) #289
Changes from 47 commits
49640db
355d5d0
f5febbd
0ac6d69
4b96dad
89a46e0
94caea2
e9c14c0
e3f2f28
1bb3c1f
f774cfb
906de70
4cee257
8a05ead
0f52ff2
34a1a53
d43335a
816c7c7
91b215c
9b3c652
717e975
04f9bc0
9bd10c4
6e0d589
b7199f6
b5d1b90
4406d9e
f0f41ce
821ae45
3bd87ca
4d0c377
5de41f6
b84107e
d3bbbd9
da34634
d594054
a533477
7db00ce
bc94676
a4cf611
7a6f6a6
946f6d7
f1b96ca
ddcbfc9
491c7a9
af46b49
daa5f6e
cd98c58
42a8b96
235a70d
6a74319
c92b1fe
b209967
8f94b7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,6 +148,34 @@ public: | |
return null; | ||
} | ||
|
||
bool resizeLarge(shared(ExtentMap)* emap, Extent* e, size_t size) shared { | ||
assert(e !is null, "Extent is null!"); | ||
assert(e.isLarge(), "Expected a large extent!"); | ||
assert(e.arenaIndex == index, "Invalid arena index!"); | ||
assert(isLargeSize(size)); | ||
|
||
// Huge is not supported: | ||
if (e.isHuge()) { | ||
return false; | ||
} | ||
|
||
auto computedPageCount = alignUp(size, PageSize) / PageSize; | ||
uint newPages = computedPageCount & uint.max; | ||
assert(newPages == computedPageCount, "Unexpected page count!"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. getPageCount |
||
|
||
auto currentSize = e.size; | ||
uint oldPages = cast(uint) (currentSize / PageSize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.pageCount |
||
|
||
// Growing is not yet supported: | ||
if (newPages >= oldPages) { | ||
return newPages == oldPages; | ||
} | ||
|
||
shrinkAlloc(emap, e, newPages); | ||
|
||
return true; | ||
} | ||
|
||
/** | ||
* Deallocation facility. | ||
*/ | ||
|
@@ -244,6 +272,32 @@ private: | |
(cast(Arena*) &this).freePagesImpl(e, n, pages); | ||
} | ||
|
||
void shrinkAlloc(shared(ExtentMap)* emap, Extent* e, uint pages) shared { | ||
assert(!e.isHuge(), "Does not support huge!"); | ||
assert(isAligned(e.address, PageSize), "Invalid extent address!"); | ||
assert(isAligned(e.size, PageSize), "Invalid extent size!"); | ||
assert(e.hpd.address is alignDown(e.address, HugePageSize), | ||
"Invalid hpd!"); | ||
assert(pages > 0 && pages <= PageCount, "Invalid number of pages!"); | ||
|
||
uint currentPages = cast(uint) (e.size / PageSize); | ||
assert(currentPages > pages, "Invalid shrink pages!"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.pageCount |
||
|
||
uint delta = currentPages - pages; | ||
|
||
uint n = ((cast(size_t) e.address) / PageSize) % PageCount; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.hpdIndex |
||
uint index = n + pages; | ||
|
||
auto newSize = pages * PageSize; | ||
emap.clear(e.address + newSize, e.size - newSize); | ||
|
||
mutex.lock(); | ||
scope(exit) mutex.unlock(); | ||
|
||
// Shrink: | ||
(cast(Arena*) &this).shrinkAllocImpl(e, index, pages, delta); | ||
} | ||
|
||
private: | ||
Extent* allocPagesImpl(uint pages, ulong mask, ExtentClass ec) { | ||
assert(mutex.isHeld(), "Mutex not held!"); | ||
|
@@ -338,6 +392,22 @@ private: | |
unusedExtents.insert(e); | ||
} | ||
|
||
void shrinkAllocImpl(Extent* e, uint index, uint pages, uint delta) { | ||
assert(mutex.isHeld(), "Mutex not held!"); | ||
assert(index <= PageCount - pages, "Invalid index!"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a blocker, but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, fixed. |
||
assert(pages > 0 && pages <= PageCount, "Invalid number of pages!"); | ||
assert(delta > 0, "Invalid delta!"); | ||
|
||
auto hpd = e.hpd; | ||
unregisterHPD(hpd); | ||
|
||
e.at(e.address, pages * PageSize, hpd); | ||
hpd.clear(index, delta); | ||
|
||
assert(!hpd.empty); | ||
registerHPD(hpd); | ||
dsm9000 marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+383
to
+390
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is great how clean this became. |
||
} | ||
|
||
/** | ||
* HugePageDescriptor heaps management. | ||
*/ | ||
|
@@ -575,3 +645,137 @@ unittest allocHuge { | |
arena.freePages(e3); | ||
arena.freePages(e4); | ||
} | ||
|
||
unittest resizeLarge { | ||
import d.gc.arena; | ||
shared Arena arena; | ||
|
||
auto base = &arena.base; | ||
scope(exit) arena.base.clear(); | ||
|
||
import d.gc.emap; | ||
static shared ExtentMap emap; | ||
emap.tree.base = base; | ||
|
||
import d.gc.region; | ||
shared RegionAllocator regionAllocator; | ||
regionAllocator.base = base; | ||
|
||
arena.regionAllocator = ®ionAllocator; | ||
|
||
// Allocation 0: 35 pages: | ||
auto ptr0 = arena.allocLarge(&emap, 35 * PageSize); | ||
assert(ptr0 !is null); | ||
auto pd0 = emap.lookup(ptr0); | ||
assert(pd0.extent.address is ptr0); | ||
assert(pd0.extent.size == 35 * PageSize); | ||
auto pd0x = emap.lookup(ptr0); | ||
assert(pd0x.extent.address is ptr0); | ||
|
||
// Allocation 1: 20 pages: | ||
auto ptr1 = arena.allocLarge(&emap, 20 * PageSize); | ||
assert(ptr1 !is null); | ||
assert(ptr1 is ptr0 + 35 * PageSize); | ||
auto pd1 = emap.lookup(ptr1); | ||
assert(pd1.extent.address is ptr1); | ||
assert(pd1.extent.size == 20 * PageSize); | ||
|
||
// Growing by zero is allowed: | ||
assert(arena.resizeLarge(&emap, pd0.extent, 35 * PageSize)); | ||
|
||
// Shrink no. 0 down to 10 pages: | ||
assert(arena.resizeLarge(&emap, pd0.extent, 10 * PageSize)); | ||
assert(pd0.extent.address is ptr0); | ||
assert(pd0.extent.size == 10 * PageSize); | ||
auto pd0xx = emap.lookup(ptr0); | ||
assert(pd0xx.extent.address is ptr0); | ||
|
||
// See that newly-last page is mapped: | ||
auto okpd = emap.lookup(ptr0 + 9 * PageSize); | ||
assert(okpd.extent !is null); | ||
|
||
// But the page after the newly-last one, should not be mapped: | ||
auto badpd = emap.lookup(ptr0 + 10 * PageSize); | ||
assert(badpd.extent is null); | ||
|
||
// Allocate 25 pages + 1 byte, will not fit in the hole after no.0: | ||
auto ptr2 = arena.allocLarge(&emap, 1 + 25 * PageSize); | ||
assert(ptr2 !is null); | ||
auto pd2 = emap.lookup(ptr2); | ||
assert(pd2.extent.address is ptr2); | ||
assert(ptr2 is ptr1 + 20 * PageSize); | ||
|
||
// Now allocate precisely 25 pages. | ||
// This new alloc WILL fit in and fill the free space after no. 0: | ||
auto ptr3 = arena.allocLarge(&emap, 25 * PageSize); | ||
assert(ptr3 !is null); | ||
auto pd3 = emap.lookup(ptr3); | ||
assert(pd3.extent.address is ptr3); | ||
assert(ptr3 is ptr0 + 10 * PageSize); | ||
|
||
arena.free(&emap, pd0, ptr0); | ||
arena.free(&emap, pd1, ptr1); | ||
arena.free(&emap, pd2, ptr2); | ||
arena.free(&emap, pd3, ptr3); | ||
|
||
// Allocate 128 pages: | ||
auto ptr4 = arena.allocLarge(&emap, 128 * PageSize); | ||
assert(ptr4 !is null); | ||
auto pd4 = emap.lookup(ptr4); | ||
assert(pd4.extent.address is ptr4); | ||
|
||
// Allocate 256 pages: | ||
auto ptr5 = arena.allocLarge(&emap, 256 * PageSize); | ||
assert(ptr5 !is null); | ||
auto pd5 = emap.lookup(ptr5); | ||
assert(pd5.extent.address is ptr5); | ||
assert(pd5.extent.hpd == pd4.extent.hpd); | ||
|
||
// Allocate 128 pages, hpd full: | ||
auto ptr6 = arena.allocLarge(&emap, 128 * PageSize); | ||
assert(ptr6 !is null); | ||
auto pd6 = emap.lookup(ptr6); | ||
assert(pd6.extent.address is ptr6); | ||
assert(pd6.extent.hpd == pd5.extent.hpd); | ||
assert(pd6.extent.hpd.full); | ||
|
||
// Shrink first alloc: | ||
assert(arena.resizeLarge(&emap, pd4.extent, 96 * PageSize)); | ||
assert(pd4.extent.size == 96 * PageSize); | ||
assert(!pd6.extent.hpd.full); | ||
|
||
// Shrink second alloc: | ||
assert(arena.resizeLarge(&emap, pd5.extent, 128 * PageSize)); | ||
assert(pd5.extent.size == 128 * PageSize); | ||
|
||
// Shrink third alloc: | ||
assert(arena.resizeLarge(&emap, pd6.extent, 64 * PageSize)); | ||
assert(pd6.extent.size == 64 * PageSize); | ||
|
||
// Allocate 128 pages, should go after second alloc: | ||
auto ptr7 = arena.allocLarge(&emap, 128 * PageSize); | ||
assert(ptr7 !is null); | ||
auto pd7 = emap.lookup(ptr7); | ||
assert(pd7.extent.address is ptr7); | ||
assert(pd7.extent.hpd == pd6.extent.hpd); | ||
assert(ptr7 is ptr5 + 128 * PageSize); | ||
|
||
// Allocate 32 pages, should go after first alloc: | ||
auto ptr8 = arena.allocLarge(&emap, 32 * PageSize); | ||
assert(ptr8 !is null); | ||
auto pd8 = emap.lookup(ptr8); | ||
assert(pd8.extent.address is ptr8); | ||
assert(pd8.extent.hpd == pd7.extent.hpd); | ||
assert(ptr8 is ptr4 + 96 * PageSize); | ||
|
||
// Allocate 64 pages, should go after third alloc: | ||
auto ptr9 = arena.allocLarge(&emap, 64 * PageSize); | ||
assert(ptr9 !is null); | ||
auto pd9 = emap.lookup(ptr9); | ||
assert(pd9.extent.address is ptr9); | ||
assert(pd9.extent.hpd == pd8.extent.hpd); | ||
assert(ptr9 is ptr6 + 64 * PageSize); | ||
|
||
// Now full again: | ||
assert(pd9.extent.hpd.full); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,12 +98,13 @@ public: | |
} | ||
} else { | ||
auto esize = pd.extent.size; | ||
if (samePointerness && alignUp(size, PageSize) == esize) { | ||
if (samePointerness && (alignUp(size, PageSize) == esize | ||
|| (isLargeSize(size) | ||
&& pd.arena.resizeLarge(emap, pd.extent, size)))) { | ||
pd.extent.setUsedCapacity(size); | ||
return ptr; | ||
} | ||
|
||
// TODO: Try to extend/shrink in place. | ||
import d.gc.util; | ||
copySize = min(size, pd.extent.usedCapacity); | ||
} | ||
|
@@ -440,10 +441,12 @@ unittest getCapacity { | |
assert(threadCache.getCapacity(p3[0 .. 20000]) == 0); | ||
assert(threadCache.getCapacity(p3[0 .. 20001]) == 20480); | ||
|
||
// This realloc happens in-place: | ||
auto p4 = threadCache.realloc(p3, 16000, false); | ||
assert(p4 !is p3); | ||
assert(p4 is p3); | ||
assert(threadCache.getCapacity(p4[0 .. 16000]) == 16384); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a test shrink from large to small? |
||
|
||
// This one, not: | ||
auto p5 = threadCache.realloc(p4, 20000, false); | ||
assert(p5 !is p4); | ||
assert(threadCache.getCapacity(p5[0 .. 20000]) == 20480); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this happen if the isLarge assert pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please see definition,
isLarge
includes any extent that isn't a slab.