Make GC triggering and heap resizing consistent #1266
Conversation
It makes more sense. We also add conversion functions between data and meta sizes.
We count size metadata in when reporting pending allocation size to the GC trigger. This ensures the MemBalancer increases the heap size by the same amount (or larger due to over-estimation) as estimated by `Space::reserved_pages`.
| @@ -313,9 +316,20 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast { | |||
| .mark_as_mapped(self.common().start, self.common().extent); | |||
| } | |||
|
|
|||
| /// Estimate the amount of side metadata memory needed for a give data memory size in pages. The | |||
| /// result will over-estimate the amount of metadata pages needed, with at least one page per | |||
There was a problem hiding this comment.
The following is not an issue from this PR, but I think it is worth bringing it up and discussing it.
Rounding up to the nearest page is okay. The real issue is that we may double count metadata memory.
Consider the case that we have 8 data pages, and the ratio between data and side metedata is 8:1. We actually have 1 side metadata page. If we call this function with 8 pages, we get 1 page -- good. But if we call this function 4 times with 2 pages each, we get 4 side metadata pages.
To estimate the side metadata pages for spaces, we go through each space, and calculate the side metadata usage for each spec in the space. The more spaces and the specs we have, the larger error we have for the calculated side metadata pages. Also if we divide the space into smaller ranges and call this function for each range, we get larger errors. This PR uses this function for pending pages, which is a relatively small memory range (compared to spaces). Luckily we do not add up the estimated pending pages, so probably it is fine.
There was a problem hiding this comment.
To estimate the side metadata pages for spaces, we go through each space, and calculate the side metadata usage for each spec in the space. The more spaces and the specs we have, the larger error we have for the calculated side metadata pages.
This is actually expected. Spaces do not overlap (Their ranges don't overlap in Map64. In Map32 with discontiguous spaces, spaces do not overlap at chunk granularity). Therefore side metadata for different spaces will not overlap. So if there are four different spaces, each haing 2 pages of data memory, and the data-meta ratio is 8:1, it will still need 4 pages of metadata, one for each space, despite that each space only needs 0.25 pages of metadata.
Also if we divide the space into smaller ranges and call this function for each range, we get larger errors. This PR uses this function for pending pages, which is a relatively small memory range (compared to spaces). Luckily we do not add up the estimated pending pages, so probably it is fine.
Yes. Pending pages are usually small, and it is almost certainly that the result will be an over-estimation. But in reality, if the allocated pending pages happen to be in a region where no side metadata has been mapped, yet, it will allocate one page in each kind of side metadata. So this kind of over-estimation covers the worst-case scenario.
But it is still arguable whether over-estimating at page granularity is the best choice. The operating system does paging at page granularity, which can be seen in /proc/pid/pagemap, as we discussed before. But MMTk does mmap at chunk granularity. Since each chunk is 4MB, that'll over-estimate the memory use to much. Page granularity happens to just work, as long as GC triggering and heap resizing are consistent with the estimation.
|
@wks Did you verify that this PR fixes the Ruby issue? |
Yes. With 100KiB heap size, it can continue from the first GC and finish the program. The heap size is 25 pages. The first GC is triggered at 26 pages (of reserved pages), and the heap size has already been adjusted to 26 pages when it triggers GC the second time. In the second GC, the reserved pages is 42 pages, 16 pages greater than 26, which means it is trying to allocate the second MarkSweep block. If I start with 104KiB of min heap size, the log will look like this: It will trigger GC when allocating the second block, and it will move on like usual until completion. |
This PR fixes a bug where the MemBalancer did not increase the heap size enough to accommodate the amount of side metadata needed by the pending allocation. It manifested as looping infinitely between triggering GC and (not actually) resizing the heap size after a GC when the minimum heap size is too small.
Now it always includes the side metadata amount when increasing heap size.
This PR also refactors the calculation of "shifting right and rounding up" which is used in multiple places. We also replace
alloc_rshiftwithlog_data_meta_ratiofor two reasons. (1) The previous implementation would cause unsigned overflow before converting the result toi32. (2)log_data_meta_ratiohas clearer semantics.