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

[Wasm][AOT] Invoking Action<struct> is 7x slower than Action<ref> #50757

Closed
jeromelaban opened this issue Apr 6, 2021 · 9 comments
Closed

[Wasm][AOT] Invoking Action<struct> is 7x slower than Action<ref> #50757

jeromelaban opened this issue Apr 6, 2021 · 9 comments

Comments

@jeromelaban
Copy link
Contributor

jeromelaban commented Apr 6, 2021

Description

Considering this code, using Mixed mode AOT without profile;

private static void TestRegisterColor()
{
    var sw = Stopwatch.StartNew();
    int value = 0;
    for (int i = 0; i < 10000000; i++)
    {
        RegisterColor(null, c => value++);
    }
    sw.Stop();
    Console.WriteLine($"Color: {sw.ElapsedMilliseconds}");
}

private static void TestRegisterOther()
{
    var sw = Stopwatch.StartNew();
    int value = 0;
    for (int i = 0; i < 10000000; i++)
    {
        RegisterOther(null, c => value++);
    }
    sw.Stop();
    Console.WriteLine($"Other: {sw.ElapsedMilliseconds}");
}

private static void RegisterColor(object o, Action<Color> action)
{
    action(new Color());
}

private static void RegisterOther(object o, Action<IDisposable> action)
{
    action(null);
}

and

public struct Color
{
  public byte A { get; set; }
  public byte B { get; set; }
  public byte G { get; set; }
  public byte R { get; set; }
}

Results are:

Color: 1710
Other: 259

Repro: https://github.com/jeromelaban/Wasm.Samples/tree/master/Bug50757/Bug50757

Configuration

b7a1648

Regression?

No

Other information

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 6, 2021
jeromelaban added a commit to jeromelaban/Wasm.Samples that referenced this issue Apr 6, 2021
@kg
Copy link
Contributor

kg commented Apr 6, 2021

I doubt it will significantly remove the gap, but maybe return an existing Color instance instead of a new one? This test case has to create a new instance and zero it (not THAT expensive, but definitely Work) then pass it as an argument value, vs the ref one just passing a nullptr.

@jeromelaban
Copy link
Contributor Author

There are workarounds for that case specifically for sure (e.g. not use a generic delegate is a very good start), yet there are many cases where this is not possible to change (in the BCL or existing nuget packages).

I'd find interesting for the sharedvt generics exclusion when running mixed-mode AOT to be configurable, as the performance impact is far from neutral.

@kg
Copy link
Contributor

kg commented Apr 6, 2021

I just meant from the perspective of "if this is fixed the two timing values should be ~equal", I absolutely agree

@ghost
Copy link

ghost commented Apr 6, 2021

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Considering this code, using Mixed mode AOT without profile;

private static void TestRegisterColor()
{
    var sw = Stopwatch.StartNew();
    int value = 0;
    for (int i = 0; i < 10000000; i++)
    {
        RegisterColor(null, c => value++);
    }
    sw.Stop();
    Console.WriteLine($"Color: {sw.ElapsedMilliseconds}");
}

private static void TestRegisterOther()
{
    var sw = Stopwatch.StartNew();
    int value = 0;
    for (int i = 0; i < 10000000; i++)
    {
        RegisterOther(null, c => value++);
    }
    sw.Stop();
    Console.WriteLine($"Other: {sw.ElapsedMilliseconds}");
}

private static void RegisterColor(object o, Action<Color> action)
{
    action(new Color());
}

private static void RegisterOther(object o, Action<IDisposable> action)
{
    action(null);
}

and

public struct Color
{
  public byte A { get; set; }
  public byte B { get; set; }
  public byte G { get; set; }
  public byte R { get; set; }
}

Results are:

Color: 1710
Other: 259

Repro: https://github.com/jeromelaban/Wasm.Samples/tree/master/Bug50757/Bug50757

Configuration

b7a1648

Regression?

No

Other information

Author: jeromelaban
Assignees: -
Labels:

area-VM-meta-mono, untriaged

Milestone: -

@CoffeeFlux
Copy link
Contributor

FYI @jeffschwMSFT most of these wasm AOT-specific issues should just get the AOT label.

@jeromelaban
Copy link
Contributor Author

As per @vargaz's help, adding -O=gsharedvt to aot-cross allows for gsharedvt code to be AOTed, and gives ~x2 performance improvement in Action<struct> invocation performance.

@SamMonoRT SamMonoRT added this to the 6.0.0 milestone Jun 15, 2021
@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label Jun 16, 2021
@SamMonoRT
Copy link
Member

@jeromelaban @vargaz - based on above comment from @jeromelaban - this seems completed ?

@SamMonoRT
Copy link
Member

Closing this, @jeromelaban please reopen if this is still an issue.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants