Page MenuHomePhabricator

mvhooren (Machiel van Hooren)
User

Projects

User does not belong to any projects.

User Details

User Since
May 1 2019, 10:26 AM (28 w, 2 d)

Recent Activity

May 14 2019

mvhooren added a comment to D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Looks good to me. Thanks so much for all your work on this Machiel!

Do you have commit access? If not I can commit on your behalf.

May 14 2019, 2:09 PM · Restricted Project

May 13 2019

mvhooren updated the diff for D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Removed changes done by clang-format that are not related to this patch.

May 13 2019, 4:08 AM · Restricted Project

May 9 2019

mvhooren updated the diff for D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Applied Clang Format to the changed files. Renamed bufferSize -> BufSize to match naming convention..

May 9 2019, 12:02 PM · Restricted Project
mvhooren updated the diff for D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Patch was missing context in the diff, updated the patch with full context.

May 9 2019, 8:02 AM · Restricted Project

May 8 2019

mvhooren added a comment to D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Looking at FileOutputBuffer, I wonder why it is using MemoryBlock/allocateMappedMemory at all. It probably should be using something like a WritableMemoryBuffer instead.
I have not changed this because it is a little beyond the scope of this patch, but it might be worth investigating. Perhaps it does have a requirement somewhere that its memory should be page-aligned, but if so it is not obvious from looking at it.

May 8 2019, 4:23 AM · Restricted Project
mvhooren added a comment to D56941: MemoryBlock: Do not automatically extend a given size to a multiple of page size..

https://reviews.llvm.org/D61599 doesn't seem to revert this, but instead adding a method to obtain an allocated size as well as a requested size (and rename size() to allocatedSize()). That's fine by me.

May 8 2019, 3:48 AM · Restricted Project
mvhooren updated the diff for D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Reverted changes from revisions r357058 and r351916 as discussed. Updated the InMemoryBuffer in FileOutputBuffer.cpp to track the requested buffer size.
The Size member of MemoryBlock and OwningMemoryBlock was renamed to AllocatedSize in order to provide a hint that the allocated size may be different than the requested size. I also added some comments to further clarify this point.
I have checked all the call sites of MemoryBlock::size() and as far as I can tell, FileOutputBuffer is the only location where wrong assumptions were made about the size.

May 8 2019, 3:43 AM · Restricted Project

May 7 2019

mvhooren added a comment to D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

Yes I agree it would be better if the Size member of MemoryBlock just held the allocated size. This would mean that r357058 and r351916 (and perhaps others) should be reverted.

I agree. To (hopefully) catch all call sites I think we will want to revert, but use your suggested "allocatedSize" name for the method. That should allow us to find and fix any call sites that are making bogus assumptions.

May 7 2019, 1:20 PM · Restricted Project
mvhooren added a comment to D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

I don't think this is the right solution. MemoryBlock's purpose is to track the allocated memory. What the client does with that memory (including subdividing it) is their business. The client already knows the number of bytes they're requesting, so the fix for fragmentation in SectionMemoryManager should be limited to fixes in SectionMemoryManager.

May 7 2019, 12:17 PM · Restricted Project

May 6 2019

mvhooren retitled D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size. from MemoryBlock objects now expose their allocated size in addition to the requested size. to [Support] MemoryBlock objects now expose their allocated size in addition to the requested size..
May 6 2019, 7:35 AM · Restricted Project
mvhooren added a comment to D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..

I do not have commit access by the way.

May 6 2019, 7:26 AM · Restricted Project
mvhooren created D61599: [Support] Reverted r357058 and r351916, MemoryBlock class now exposes its allocated size instead of requested size..
May 6 2019, 7:25 AM · Restricted Project