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

[GC] Add option G1BarrierSimple to use simple g1 post barrier #59

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

mmyxym
Copy link
Collaborator

@mmyxym mmyxym commented Jun 7, 2024

Summary: Provide option G1BarrierSimple to use simple G1 post barrier for better mutator performance

Testing: CI pipeline

Reviewers: yude, yifeng

Issue: #56

@CLAassistant
Copy link

CLAassistant commented Jun 7, 2024

CLA assistant check
All committers have signed the CLA.

@mmyxym mmyxym force-pushed the g1barriersimple branch 4 times, most recently from d7ad65a to de0e103 Compare June 12, 2024 06:54
if (G1BarrierSimple) {
for (; byte <= last_byte; byte++) {
*byte = G1CardTable::dirty_card_val();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use memset(), like what we did in dragonwell11?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


__ load_byte_map_base(scratch);
__ add(start, start, scratch);
__ bind(L_loop);
Copy link
Collaborator

Choose a reason for hiding this comment

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

assert(dirty_card_val == 0)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

__ mov64(tmp, disp);
__ addptr(addr, tmp);
__ bind(L_loop);
__ movb(Address(addr, count, Address::times_1), 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use G1CardTable::dirty_card_val() like the rest of the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -98,6 +100,36 @@ void G1BarrierSetAssembler::gen_write_ref_array_pre_barrier(MacroAssembler* masm

void G1BarrierSetAssembler::gen_write_ref_array_post_barrier(MacroAssembler* masm, DecoratorSet decorators,
Register addr, Register count, Register tmp) {
#ifdef _LP64
Copy link
Collaborator

Choose a reason for hiding this comment

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

G1BarrierSimple seems to permit X86. What happens on that platform?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The option will be ignored.

@linade
Copy link
Collaborator

linade commented Jun 14, 2024

Probably not enough functional tests which will concern our coverage goals..

Summary: Provide option G1BarrierSimple to use simple G1 post barrier for better mutator performance

Testing: CI pipeline

Reviewers: yude, yifeng

Issue: dragonwell-project#56
@@ -177,6 +177,25 @@ void G1Arguments::initialize() {
FLAG_SET_ERGO(ParallelGCThreads, 1);
}

if (G1BarrierSimple) {
#if !defined(_LP64) || !(defined(X86) || defined(AARCH64))
Copy link
Collaborator

Choose a reason for hiding this comment

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

X86 is allowed here?

Copy link
Collaborator Author

@mmyxym mmyxym Jun 17, 2024

Choose a reason for hiding this comment

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

x86 32bit is not supported.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, took me a while to see this means !(x86_64 || aarch64)

@mmyxym mmyxym merged commit 693ec2e into dragonwell-project:master Jun 19, 2024
90 of 91 checks passed
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.

None yet

4 participants