Add Compiler Memory Manager Documentation #2003
Conversation
doc/compiler/memory/MemoryManager.md
Outdated
|
|
||
| The OMR Compiler doesn't necessarily operate in a vacuum, but rather | ||
| as one component of a Managed runtime. Additionally, communications | ||
| between components could be extern "C". Therefore, one can't simply |
There was a problem hiding this comment.
Instead of saying "extern "C"" perhaps say "could be using C linkage"? If we do keep "extern "C"", it should be preformatted with backticks
doc/compiler/memory/MemoryManager.md
Outdated
| use something like `::operator new` since it can throw. Additionally, | ||
| mutli-threaded performance of malloc historically has been hit or miss, | ||
| and any generic common allocator would have poor interaction between | ||
| components, with little to no insight into usage. |
There was a problem hiding this comment.
sp. multi
I don't think throwing shade at malloc here is a good call - If malloc's threaded performance were really at issue, there would be other allocators we could be deploying instead. The fact we don't is because we believe that we can derive substantial gains in both performance and functionality by having a compiler-domain-specific allocator.
(I object mildly to the description of the port allocator as 'domain-specialized', because in the context of the compiler, it is precisely not domain specific)
There was a problem hiding this comment.
I reworded the line regarding the port allocator to
Using an allocator specific to a Managed Runtime, such as
omrmem_allocate_memoryin the Port Library...
Regarding the comment about malloc, I think the point there is that from (our) historical empirical observation, malloc has been wasteful for memory when there are multiple simultaneous compilations occurring (since it uses per thread arenas + a global arena); it also results in more fragmentation since memory allocated in a compilation gets freed at the end of the compile but malloc doesn't release that memory back to the OS. In terms of other allocators we could be deploying, that's addressed by the line
any generic common allocator would have poor interaction between components, with little to no insight into usage.
So it's not so much that we're putting down malloc so much as we're stating that in certain compiler configurations, malloc ends up hurting more than helping.
I can update the doc to be more explicit about what "hit or miss" means, or I can still take it out if you think it doesn't belong there.
There was a problem hiding this comment.
:D I think putting in the above is a pretty good call -- but at the end of the day, we have a story where we believe we can do pretty well for our allocation patterns by taking matters into our own hands.
There was a problem hiding this comment.
Re: shade on malloc. I thing the odds are high that a large part of why we went down this road is because malloc's historical multi-threaded performance was awful. A lot of this work started when dlmalloc was still the leading implementation. I have huge respect for dlmalloc, but it is not thread friendly. Whether or not the historical context belongs in document describing current practice is 🤷 from me.
malloc's multi-threaded performance notwithstanding. The reason this is all justified today, is that the compiler produces a prolific amount of garbage during a compilation, and if it were all allocated individually from a generic allocator, we'd be at serious risk of long-term memory fragmentation. That's on top of any heavy-weight RAS requirements for a common runtime allocator.
doc/compiler/memory/MemoryManager.md
Outdated
| Therefore, the compiler manages memory its own memory. | ||
|
|
||
|
|
||
| ## Compiler Memory Manager Hierarchy |
There was a problem hiding this comment.
IMO, leading this description with an ASCII-art diagram of the hierarchies would be a great choice.
doc/compiler/memory/MemoryManager.md
Outdated
| `TR::PersistentAllocator`. The `TR::Region` would then be provided this | ||
| `TR::PersistentSegmentProvider` object. | ||
|
|
||
| `TR::Region` allocations are untyped raw memory. In order to have a region |
There was a problem hiding this comment.
I would actually hoist some of the information about destructor semantics to near the top of the document: It's a peculiar facet of our use of memory and so should be highlighted earlier.
There was a problem hiding this comment.
I can bring that information up, but the concern I have is that a reader is not going to know what it all means until the read the rest of the document. It currently also flows a bit better, since the doc starts with the the motivation, the hierarchy, how the compiler uses memory, and then subtleties and information. Personally I would find it confusing if I saw information at the top that only made sense after reading the whole document, since I would probably not read in order but skip over to the section where a particular term is first described. However, if you think the average reader would be fine with it on top, then I can move it to the top.
There was a problem hiding this comment.
I guess I'm torn a bit.
Maybe surround it with 🚨 emojis to call attention to the fact constructors do not fire by default. That's the thing I think should be made clear as possible.
There was a problem hiding this comment.
+1 for highlighting the nasty gotyas inherent in naive slab allocators.
[ci skip] Signed-off-by: Irwin D'Souza <dsouzai@ca.ibm.com>
dsouzai
force-pushed
the
memoryDocumentation
branch
from
e65275c
to
9861d8a
Compare
|
@mgaudet updated the document with your requested changes :) |
|
Looks good to me! 🚨 |
|
@0xdaryl This should be good to merge if everything looks good to you. |
No description provided.