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

Fix unaligned edge access #232

Merged
merged 2 commits into from
Aug 22, 2023
Merged

Fix unaligned edge access #232

merged 2 commits into from
Aug 22, 2023

Conversation

wks
Copy link
Contributor

@wks wks commented Aug 21, 2023

This allows the OpenJDK binding to load and store ObjectReferences from/to edges at unaligned addresses on x86 and x86_64.

Fixes: #231

@wks wks requested review from qinsoon and wenyuzhao August 21, 2023 08:28
We load/store edges using ptr::{read,write}_unaligned on x86 and x86_64
to workaround unaligned edges in code roots.
@wks wks force-pushed the fix/unaligned-edge-access branch from c677418 to 75d4809 Compare August 21, 2023 09:58
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. Thanks.

@wks
Copy link
Contributor Author

wks commented Aug 22, 2023

https://squirrel.anu.edu.au/plotty/wks/noproject/#0|bobcat-2023-08-21-Mon-135824&benchmark^build^invocation^iteration^mmtk_gc&GC^time^time.stw&|10&iteration^1^4|20&1^invocation|30&1&mmtk_gc&build;build1|41&Histogram%20(with%20CI)^build^mmtk_gc&

I ran lusearch, 100 iterations, 2.5x min heap size, on bobcat.moma. Build1 is master and build2 is this PR.

The difference in STW time is less than 1%. There seems to be a very small amount of performance improvement, but is within the confidence interval.

image

@wks
Copy link
Contributor Author

wks commented Aug 22, 2023

The generated assembly for ProcessEdgesWork::do_work is identical between build1 and build2. I think the difference is a result of noise or subtle differences in unrelated code.

; r14: self; rax: buffer
  109ca1:	49 8b 46 08          	mov    0x8(%r14),%rax      ; let buffer = self.edges.buf;
; rbx: index; r13: edge
  109ca5:	4c 8b 2c d8          	mov    (%rax,%rbx,8),%r13  ; let edge = buffer[index * 8];
  109ca9:	48 ff c3             	inc    %rbx                ; index += 1
; rdx: object
  109cac:	49 8b 55 00          	mov    0x0(%r13),%rdx      ; let object = edge.addr.as_ptr().read_unaligned();
  109cb0:	48 85 d2             	test   %rdx,%rdx           ; if object.is_null() {
  109cb3:	75 cb                	jne    109c80 <mmtk::scheduler::gc_work::<impl mmtk::scheduler::work::GCWork<<E as mmtk::scheduler::gc_work::ProcessEdgesWork>::VM> for E>::do_work+0x30>

@wks wks merged commit 129c889 into mmtk:master Aug 22, 2023
44 of 45 checks passed
tianleq pushed a commit to tianleq/mmtk-openjdk that referenced this pull request Apr 3, 2024
We load/store edges using ptr::{read,write}_unaligned on x86 and x86_64
to workaround unaligned edges in code roots.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misaligned pointer dereference after moving to Rust 1.71.1
3 participants