-
Notifications
You must be signed in to change notification settings - Fork 69
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
Improve nogc error when out of space #1175
base: master
Are you sure you want to change the base?
Conversation
When using `NoGC` if the machine runs out of memory, the user will see an error that says garbage collection failed: ``` ERROR: An MMTk GC thread panicked. This is a bug. panicked at /Users/eileencodes/src/github.com/open_source/mmtk-core/src/plan/nogc/global.rs:74:9: internal error: entered unreachable code: GC triggered in nogc Backtrace is disabled. run with `RUST_BACKTRACE=1` environment variable to display a backtrace running file: test/zlib/test_zlib.rb ``` I added a check in `poll` to see if the plan being used supports GC and if not, raise an error. I could have updated the `schedule_collection` function to return this error, but I felt that since that function is regarding scheduling, it was better to raise if any plan does not support GC than change the purpose of `schedule_collection`. We still want to know if a GC ended up being triggered in `schedule_collection`. The new error looks like this: ``` [2024-07-24T18:43:45Z INFO mmtk::memory_manager] Initialized MMTk with NoGC (FixedHeapSize(1073741824)) thread '<unnamed>' panicked at /Users/eileencodes/src/github.com/open_source/mmtk-core/src/util/heap/gc_trigger.rs:80:17: internal error: entered unreachable code: User ran out of space and the plan does not support collecting garbage. note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fatal runtime error: failed to initiate panic, error 5 running file: test/zlib/test_zlib.rb ```
Yes. It makes sense to let it exit immediately when a mutator thread has an allocation failure. The previous code relies on the I made some slight changes. I retained the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There are two additional problems.
I think we need to think more about this topic, i.e. out-of-memory handling in NoGC. Intuitively, if we run out of memory in NoGC, then |
We can just rewrite related tests. I am also okay if you would like to call |
When using
NoGC
if the machine runs out of memory, the user will see an error that says garbage collection failed:I added a check in
poll
to see if the plan being used supports GC and if not, raise an error. I could have updated theschedule_collection
function to return this error, but I felt that since that function is regarding scheduling, it was better to raise if any plan does not support GC than change the purpose ofschedule_collection
. We still want to know if a GC ended up being triggered inschedule_collection
.The new error looks like this:
Note: I've written very little rust so while I know this works (tested in CRuby) I don't know if from a design perspective this is correct.