Implement fixed heap size for Julia #38
Conversation
There was a problem hiding this comment.
Mostly SGTM.
The global counter for malloc may introduce too much contention on the corresponding cache line on allocation-heavy multithreaded workloads which increment this counter at a high rate.
If MMTk also does that then it's probably an apples-to-apples comparison.
Thanks for reviewing the PR. Yeah. I was concerned about this as well. MMTk/Julia uses a global counter like this, so for comparison, it is fine. But I will try measure the overhead. If this is a source of slowdown for MMTk/Julia, we should know about it. |
It does have some measurable overhead. See mmtk/mmtk-julia#141. |
|
@udesou Should we get this merged? Also let me know if you think we should have this change in |
I think so. I believe it doesn't impact stock if we don't have a fixed heap limit set, right? As in, standard stock remains the same. It would be nice to have it merged to make it more available and integrated with future changes we have, when we're testing it. |
This impacts stock even if we don't use a fixed heap limit: 1. it adds a global counter for malloc, and it will slow down workloads that use malloc frequently. 2. it adds a check in the I can turn fixed heap size into a build-time option. In that case, there will be no overhead if we don't enable it in the build. |
qinsoon
force-pushed
the
fixed-heap-size
branch
from
068fb2d
to
e29cbc2
Compare
|
@udesou I have added a build-time flag |
| @@ -574,6 +584,18 @@ void gc_setmark_buf(jl_ptls_t ptls, void *o, uint8_t mark_mode, size_t minsz) JL | |||
|
|
|||
| inline void maybe_collect(jl_ptls_t ptls) | |||
| { | |||
| #ifdef GC_FIXED_HEAP | |||
| if (jl_options.fixed_heap_size) { | |||
There was a problem hiding this comment.
Do you still need these internal ifs if you have defined GC_FIXED_HEAP? Aren't they redundant?
There was a problem hiding this comment.
For a build with GC_FIXED_HEAP, you can still run with the stock heuristics if fixed_heap_size is not set. If you think it is a better idea to force using fixed_heap_size, I can change that.
There was a problem hiding this comment.
I see, you're building fixed heap support when you use GC_FIXED_HEAP.
But if you do build with GC_FIXED_HEAP and use the stock heuristics you would pay the performance overhead you mentioned above right? Then you probably want to force using fixed_heap_size when you build with GC_FIXED_HEAP, or even add a warning to make it clear.
There was a problem hiding this comment.
I added a warning. I tried to make it a hard error. But the building process uses julia without setting a heap size. So better also keep the stock heuristics in place.
This PR introduces fixed heap size for stock Julia. With the build time option
WITH_GC_FIXED_HEAP=1and using--fixed-heap-size=..., it will bypass all the existing GC triggering heuristics, and only do GC when the heap size reaches the defined heap size, and will only do a full heap GC if the free memory after a GC is less than 20% of the heap size.This PR also introduces a global counter for mallocd bytes. This will slow down the performance of malloc. For MMTK Julia, we also use such a counter (see mmtk/mmtk-julia#141). I plan to do another PR to fix this for both MMTK Julia and stock Julia.