Skip to content

Conversation

no-defun-allowed
Copy link
Collaborator

This PR adds regions to the Compressor. The benefits are that the collector compacts each region in parallel, and the collector supports the use of discontiguous heaps (as required for compressed oops in OpenJDK).

@no-defun-allowed
Copy link
Collaborator Author

no-defun-allowed commented Aug 21, 2025

Just to clarify, how is a RegionPageResource different from other existing page resources?

I think your understanding is right. To elaborate, the RegionPageResource structures the heap into regions, and has a bump allocator for each region. The RegionPageResource can handle requests for any number of pages smaller than a region; but requests probably should be smaller than the region size by a decent margin, to avoid fragmentation. The regions in the Compressor are 1MiB, though that size was picked arbitrarily, and I don't know if that size is a good size for any idea of "good".

RegionPageResource allows the GC design to assume that objects never span multiple regions when they are allocated, which is necessary to compact each region separately; and that the objects in a region will be contiguously allocated, which we utilise in only scanning for objects between the start and allocation cursor of a region (though I haven't measured how much this matters). The Compressor and RegionPageResource together also maintain allocation order in each region.

@no-defun-allowed no-defun-allowed marked this pull request as ready for review August 22, 2025 06:18
@qinsoon qinsoon self-requested a review August 25, 2025 05:29
@qinsoon qinsoon added the PR-extended-testing Run extended tests for the pull request label Aug 25, 2025
@qinsoon
Copy link
Member

qinsoon commented Aug 25, 2025

Just out of curiosity, what is performance like for this PR? It makes the offset calculation and the actual copying parallelized. I would expect it to improve STW time.

@no-defun-allowed
Copy link
Collaborator Author

Just out of curiosity, what is performance like for this PR? It makes the offset calculation and the actual copying parallelized. I would expect it to improve STW time.

I don't have apples-to-apples results handy - I have benchmark results with compressed oops for the regional Compressor, but none without compressed oops.

@k-sareen
Copy link
Collaborator

Just out of curiosity, what is performance like for this PR? It makes the offset calculation and the actual copying parallelized. I would expect it to improve STW time.

I don't have apples-to-apples results handy - I have benchmark results with compressed oops for the regional Compressor, but none without compressed oops.

The comparisons against SemiSpace, Immix, etc. would still be interesting to see. I don't think Yi would have seen those results.

@no-defun-allowed
Copy link
Collaborator Author

no-defun-allowed commented Aug 26, 2025

Just out of curiosity, what is performance like for this PR? It makes the offset calculation and the actual copying parallelized. I would expect it to improve STW time.

I don't have apples-to-apples results handy - I have benchmark results with compressed oops for the regional Compressor, but none without compressed oops.

The comparisons against SemiSpace, Immix, etc. would still be interesting to see. I don't think Yi would have seen those results.

Right. I have such comparisons on plotty: http://squirrel.anu.edu.au/plotty/hayleyp/plots/p/CDBtDw

(Edited because I put the wrong logs in - I re-benchmarked with the minheaps for Compressor, as those are the smallest.)

@wks
Copy link
Collaborator

wks commented Aug 26, 2025

Is the main reason for introducing RegionPageResource the ability to enumerate regions? It holds a Vec<RegionAllocator<R>> so that you can iterate through it and process multiple regions in parallel.

But if CompressorSpace is backed by a MonotonicPageResource, you can already iterate using iterate_allocated_regions. Each "allocated region" is one chunk obtained from VMMap (which can be either Map32 or Map64), and it will always be exactly one chunk because no spaces except LOS can allocate objects larger than a chunk. All chunks will be almost full, and the last chunk will be filled up to MonotonicPageResourceSync::cursor. Because each chunk is exactly 4MB, you can always divide it into integer multiples of "regions" chosen by CompressorSpace (in the case where each CompressorRegion is 1MB, each chunk can be split into 4 regions). Then you can wrap the iterator implemented in MonotonicPageResource::iterate_allocated_regions to make your "region iterator", and parallelize the compaction at the region granularity, if chunk-grained parallelism is not good enough.

In this way, you don't really need the RegionPageResource or a Vec<RegionAllocator<R>>. You can just use MonotonicPageResource, and iterate over 1MB "virtual" regions.

@wks
Copy link
Collaborator

wks commented Aug 26, 2025

I see. It never moves objects between regions. So we need to keep one cursor for every region. In the end, we will have many regions, and the live objects in each region are pushed to the beginning, something like this:

LLLLL........... LL.............. LLLLLLLLLLL..... ................ LLLLLLLLLLLLLLL.

So we need one cursor per region to identify where we can continue allocating.

@no-defun-allowed
Copy link
Collaborator Author

It never moves objects between regions.

So we need one cursor per region to identify where we can continue allocating.

Exactly.

@qinsoon
Copy link
Member

qinsoon commented Aug 28, 2025

The test fail in Julia should be unrelated with this PR.

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@k-sareen
Copy link
Collaborator

We should make a separate PR for the OpenJDK binding to remove the -XX:-UseCompressedOops from the CI for the Compressor.

@k-sareen
Copy link
Collaborator

binding-refs
OPENJDK_BINDING_REPO=k-sareen/mmtk-openjdk
OPENJDK_BINDING_REF=enable-compressed-oops-compressor

@qinsoon qinsoon added this pull request to the merge queue Sep 8, 2025
Merged via the queue into mmtk:master with commit a061e05 Sep 8, 2025
41 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-extended-testing Run extended tests for the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants