Conversation
0d1c7d3 to
a5035a2
Compare
|
Here are some preliminary benchmark results. Two builds:
I ran it with
From the data, the impact on total time is between -0.5% to +0.5%, and the error bar is large. Interestingly, it always shows slowdown on shrew, and always speed-up on vole. For STW time, "shrew 2.5x" has a slow-down of 1%, and "vole 3x" has a speed-up of 1.5%, but their error bars are too large. I think the result of forcing |
|
More benchmark results. I ran the same binary on shrew.moma and mole.moma (identical to vole.moma), 20 invocations, 3x min heap size, using Immix and StickyImmix GC. Total time: normalized to build1 of the corresponding machine-plan pair: For each machine-plan pair, the geomean of total time of build2 is between 99.6% to 100.3% of that of build1. Other time: Geomeans are between 99.8% to 100.3% of build1. STW time: STW times are between 97.9% to 101.0% of build1. But given the noise level, it is hard to conclude there is any improvement. We can see significant speed-up for build2-shrew-StickyImmix for biojava, jython and pmd, but build2-mole-StickyImmix does not have the same speed-up for those benchmarks. This is a bit hard to explain. |
|
binding-refs |
|
This PR is now ready for review. Associated PRs for the bindings: |
qinsoon
left a comment
There was a problem hiding this comment.
Looks good to me. There are some minor points.
Also a quick check: is there any change that may affect performance for OpenJDK and MMTk since the evaluation in #1064 (comment)?
| #[repr(transparent)] | ||
| #[derive(Copy, Clone, Eq, Hash, PartialOrd, Ord, PartialEq, NoUninit)] | ||
| pub struct ObjectReference(usize); | ||
| pub struct ObjectReference(NonZeroUsize); |
There was a problem hiding this comment.
The comments for the type should note that ObjectReference cannot be null, and for places where we may have an invalid object reference, we use Option<ObjectReference>. NonZeroUsize makes sure that Option<ObjectReference> has the same length as ObjectReference.
src/util/mod.rs
Outdated
| // This module is made public so the binding could implement allocator slowpaths if they would like to. | ||
| pub mod alloc; | ||
| /// Helpers for making native APIs. | ||
| pub mod apiutils; |
There was a problem hiding this comment.
Could use the name api_util to be consistent with other existing modules such as rust_util/test_util.
I don't think so, but I'll re-run the benchmarks just to be safe. |
|
I re-ran the benchmarks with the same setting as in #1064 (comment) Total time: (geomens of build2 are between 99.5% to 100.2% of build1, varying among
Other time: (99.6% to 100.1%)
STW time: (99.2% to 100.8%)
The differences are within 1%. This result is consistent with the last evaluation. |
There were a few comments that still mentioned null ObjectReference. They have been removed.
Parent PR: mmtk/mmtk-core#1064 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Parent PR: mmtk/mmtk-core#1064 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Upstream PR: mmtk/mmtk-core#1064 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Upstream PR: mmtk/mmtk-core#1064 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Parent PR: mmtk/mmtk-core#1064 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Parent PR: mmtk/mmtk-core#1064 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
We changed
ObjectReferenceso that it is backed byNonZeroUsize, and can no longer represent a NULL reference. For cases where anObjectReferencesmay or may not exist,Option<ObjectReference>should be used instead.This is an API-breaking change.
ObjectReference::NULLandObjectReference::is_null()are removed.ObjectReference::from_raw_address()now returnsOption<ObjectReference>. The unsafe methodObjectReference::from_raw_address_unchecked()assumes the address is not zero, and returnsObjectReference.ObjectReferencenow returnOption<ObjectReference>. Examples includeEdge::load()andReferenceGlue::get_referent().If a VM binding needs to expose
Option<ObjectReference>to native (C/C++) programs, a convenient typecrate::util::api_util::NullableObjectReferenceis provided.Fixes: #1043