Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SFTDenseChunkMap on 64bits when vm_space is enabled #1094

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Mar 20, 2024

This PR resolves the issues found in mmtk/mmtk-julia#143.

[2024-03-19T05:23:33Z INFO  mmtk::policy::vmspace] Set [78624DC00000, 786258000000) as VM region (heap available range [20000000000, 220000000000))
thread '<unnamed>' panicked at 'start = 78624DC00000 + bytes = 171966464 should be smaller than space_start 380000000000 + max extent 2199023255552, index 28, table size 31', /home/runner/.cargo/git/checkouts/mmtk-core-89cdc7bf360cce7f/46b0abe/src/policy/sft_map.rs:202:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5

Basically we should not use SFTSpaceMap when we have off-heap memory. Julia tries to set VM space at an address range outside our heap range, and that causes issues when we compute index for SFTSpaceMap. Using SFTDenseChunkMap will solve the issue.

Changes:

@k-sareen
Copy link
Collaborator

I presume 32-bit compressed pointer VMs are unaffected? (Selfish question given ART is 32-bit and has a bootimage)

@qinsoon
Copy link
Member Author

qinsoon commented Mar 20, 2024

I presume 32-bit compressed pointer VMs are unaffected? (Selfish question given ART is 32-bit and has a bootimage)

Right, 32 bits are not affected. I will update the title to make it clearer.

@qinsoon qinsoon changed the title Use SFTDenseChunkMap when vm_space is enabled Use SFTDenseChunkMap on 64bits when vm_space is enabled Mar 20, 2024
@qinsoon qinsoon added the PR-extended-testing Run extended tests for the pull request label Mar 20, 2024
@qinsoon qinsoon requested a review from wks March 21, 2024 05:18
@qinsoon qinsoon marked this pull request as ready for review March 21, 2024 05:18
src/policy/sft_map.rs Show resolved Hide resolved
src/policy/sft_map.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

LGTM, but this PR needs rebasing.

@qinsoon qinsoon enabled auto-merge March 21, 2024 09:20
@qinsoon qinsoon added this pull request to the merge queue Mar 21, 2024
Merged via the queue into mmtk:master with commit dd84218 Mar 21, 2024
24 checks passed
@qinsoon qinsoon deleted the different-sft-map-for-vm-space branch March 21, 2024 11:16
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.

3 participants