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

JIT: Bulk copy of byrefs #101761

Merged
merged 5 commits into from
May 3, 2024
Merged

JIT: Bulk copy of byrefs #101761

merged 5 commits into from
May 3, 2024

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented May 1, 2024

Closes #101699
Closes #8627

Use a bulk copy when we need to copy a struct with gc fields (non-null) instead of individual gc write barriers

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(MyBench).Assembly).Run(args);

public class MyBench
{
    GcStruct4 s4 = new GcStruct4(
        100001.ToString(), 100002.ToString(),
        100003.ToString(), 100004.ToString());

    GcStruct9 s9 = default;

    [Benchmark] public object BoxS4() => s4;

    [Benchmark] public object BoxS9() => s9;
}

record struct GcStruct4(
    string a1, string a2, 
    string a3, string a4);

record struct GcStruct9(
    string a1, string a2, string a3, 
    string a4, string a5, string a6,
    string a7, string a8, string a9);
Method Toolchain Mean Ratio
BoxS4 \runtime-main...\corerun.exe 8.160 ns 1.00
BoxS4 \runtime-pr...\corerun.exe 5.254 ns 0.64
BoxS9 \runtime-main...\corerun.exe 13.546 ns 1.00
BoxS9 \runtime-pr...\corerun.exe 6.136 ns 0.45

@EgorBo
Copy link
Member Author

EgorBo commented May 1, 2024

@MihuBot

@EgorBo
Copy link
Member Author

EgorBo commented May 1, 2024

@MihuBot

@EgorBo EgorBo force-pushed the bulk-assignbyref branch from 97b1d70 to efa0292 Compare May 1, 2024 23:02
@EgorBo
Copy link
Member Author

EgorBo commented May 1, 2024

@MihuBot

@EgorBo
Copy link
Member Author

EgorBo commented May 2, 2024

@jkotas PTAL

@EgorBo EgorBo requested a review from jkotas May 2, 2024 01:46
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@EgorBo
Copy link
Member Author

EgorBo commented May 2, 2024

@jakobbotsch cc @dotnet/jit-contrib PTAL jit part, TryLowerBlockStoreAsGcBulkCopyCall method is based on the tricks we used in LowerBlockStoreAsHelperCall (morphing calls in lower).

Comment on lines 8239 to 8244
if (data->OperIs(GT_IND))
{
// Drop GT_IND nodes
BlockRange().Remove(data);
data = data->AsIndir()->Addr();
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to worry about any indir flags here, like GTF_IND_VOLATILE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, I think it's safer to just give up on those here, fixed.

@dotnet dotnet deleted a comment from EgorBot May 3, 2024
@EgorBo
Copy link
Member Author

EgorBo commented May 3, 2024

Failures are known - #101721, #101618

@EgorBo EgorBo merged commit 7ac5ef9 into dotnet:main May 3, 2024
148 of 152 checks passed
@EgorBo EgorBo deleted the bulk-assignbyref branch May 3, 2024 15:54
@dotnet dotnet deleted a comment from EgorBot May 3, 2024
michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request May 9, 2024
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants