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.

Sign up for GitHub

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

Jump to bottom

Check the assertion failure in SFT map #143

Closed
wants to merge 3 commits into from
Closed

Check the assertion failure in SFT map #143

wants to merge 3 commits into from

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Mar 19, 2024

No description provided.

@qinsoon qinsoon mentioned this pull request Mar 20, 2024
Merged
@wks
Copy link
Contributor

wks commented Mar 21, 2024

The error message contains note: run with RUST_BACKTRACE=1 environment variable to display a backtrace. It'll be better if we can set RUST_BACKTRACE=1 when running CI tests so that it always prints stack traces.

github-merge-queue bot pushed a commit to mmtk/mmtk-core that referenced this pull request Mar 21, 2024
@qinsoon
Use SFTDenseChunkMap on 64bits when vm_space is enabled (#1094)
dd84218 
This PR resolves the issues found in
mmtk/mmtk-julia#143.
```console
[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:
* Use `SFTDenseChunkMap` if `vm_space` is enabled.
* Add a range check for `SFTSpaceMap::has_sft_entry()`. The
address/index computation is only correct if the address is in the
range.
* Add a simple test case for
mmtk/mmtk-julia#143.
@qinsoon
Copy link
Member Author

qinsoon commented Mar 27, 2024

This is the performance results for this PR. It shows that there are two benchmarks that are significantly slowed down by more than 10% while most benchmarks are observed with a slowdown of a few percents. Though this PR (along with the mmtk-core PR) fixes a correctness bug (using SFTSpaceMap with off-heap VM space is wrong), the performance is not satisfactory. I will look further into this.

total time total time error gc time mutator time
('multithreaded-big_arrays-issue-52937', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 0.997136 0.00268769 0 0.997136
('multithreaded-big_arrays-objarray', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.01231 0.0114875 1.03808 1.00693
('multithreaded-binary_tree-tree_immutable', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.03907 0.00656719 1.07512 1.00402
('multithreaded-binary_tree-tree_mutable', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.04052 0.00628581 1.08186 1.00477
('multithreaded-mergesort_parallel-mergesort_parallel', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.00196 0.00632791 0 1.00196
('multithreaded-mm_divide_and_conquer-mm_divide_and_conquer', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.0079 0.0161512 0 1.0079
('serial-TimeZones-TimeZones', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.04728 0.00651527 0 1.04728
('serial-append-append', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.09984 0.0190819 1.12163 0.999188
('serial-big_arrays-many_refs', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 0.989741 0.0245686 0.990786 0.986718
('serial-big_arrays-single_ref', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 0.989843 0.0296685 0.989805 0.990089
('serial-bigint-pollard', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.10983 0.0280107 1.16513 1.0113
('serial-linked-list', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.00937 0.00812906 0 1.00937
('serial-linked-tree', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.00088 0.00298534 0 1.00088
('serial-strings-strings', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 0.993816 0.00514627 0 0.993816
('slow-bigint-pidigits', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 1.14245 0.007889 1.149 1.01848
('slow-rb_tree-rb_tree', 'julia-mmtk-pr(1.5x minheap,.mmtk_gc_threads-12)') 0.999169 0.000889877 0.97978 0.999665

@qinsoon
Copy link
Member Author

qinsoon commented Apr 29, 2024

#147 tracks the performance slowdown. I will close this issue.

@qinsoon qinsoon closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@qinsoon @wks