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] Performance regression in List<string> foreach #54272

Closed
jeromelaban opened this issue Jun 16, 2021 · 9 comments
Closed

[Wasm][AOT] Performance regression in List<string> foreach #54272

jeromelaban opened this issue Jun 16, 2021 · 9 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-Codegen-AOT-mono
Milestone

Comments

@jeromelaban
Copy link
Contributor

jeromelaban commented Jun 16, 2021

Description

A performance regression happened when using this repro (with -O=gsharedvt for mono-aot-cross):

var SUT = new List<object>();
SUT.AddRange(Enumerable.Range(1, 1000)
	.Select(i => (object)null));

var sw = Stopwatch.StartNew();

for (int i = 0; i < 5000; i++)
{
	int count = 0;
	foreach (var item in SUT)
	{
		count++;
	}
}

sw.Stop();
Console.WriteLine($"TestForeachListOfT: {sw.Elapsed}");

Some performance benchmarks:

With df6e956:
Result: TestForeachListOfT: 00:00:00.3979650

image

With a822d39:
Result: TestForeachListOfT: 00:00:00.1164150
image

With d07f911 and 337216a:
Result: TestForeachListOfT: 00:00:00.2462100
image

Configuration

Regression?

Yes

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 Jun 16, 2021
@jeromelaban jeromelaban changed the title [Wasm][AOT] Regression in List<string> foreach [Wasm][AOT] Performance regression in List<string> foreach Jun 16, 2021
@radical radical added the arch-wasm WebAssembly architecture label Jun 16, 2021
@ghost
Copy link

ghost commented Jun 16, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

A performance regression happened when using this repro (with -O=gsharedvt for mono-aot-cross):

var SUT = new List<object>();
SUT.AddRange(Enumerable.Range(1, 1000)
	.Select(i => (object)null));

var sw = Stopwatch.StartNew();

for (int i = 0; i < 5000; i++)
{
	int count = 0;
	foreach (var item in SUT)
	{
		count++;
	}
}

sw.Stop();
Console.WriteLine($"TestForeachListOfT: {sw.Elapsed}");

Some performance benchmarks:

With df6e956:
Result: TestForeachListOfT: 00:00:00.3979650

image

With a822d39:
Result: TestForeachListOfT: 00:00:00.1164150
image

With d07f911 and 337216a:
Result: TestForeachListOfT: 00:00:00.2462100
image

Configuration

Regression?

Yes

Other information

Author: jeromelaban
Assignees: -
Labels:

arch-wasm, untriaged

Milestone: -

@vargaz
Copy link
Contributor

vargaz commented Jun 25, 2021

This should be fixed by
d4fc723

@jeromelaban
Copy link
Contributor Author

Here's the perf numbers, with 881e902:

TestForeachListOfT: 00:00:00.3946000
dotnet.js:2298 TestForListOfT: 00:00:00.0623000
dotnet.js:2298 TestForArray: 00:00:00.0212000

Still with -O=gsharedvt, it looks like the situation has not improved.

@vargaz
Copy link
Contributor

vargaz commented Jun 26, 2021

What is the issue here ? Is it that foreach on a list is slower than a for loop or a for loop on an array ?

@jeromelaban
Copy link
Contributor Author

The issue is that the foreach loop is calling into interpreted code, where as the for loop is not. When the code in the method is AOTed, it's 3x faster than the interpreted one (see the flame chart for a822d39 above).

@jeromelaban
Copy link
Contributor Author

jeromelaban commented Jun 26, 2021

Here's the full sample I'm running:

static void Main(string[] args)
{
	TestEnumeratorListOfT();
	TestForeachListOfT();
	TestForListOfT();
	TestForArray();
}

private static void TestForArray()
{
	var SUT = new List<object>();
	SUT.AddRange(Enumerable.Range(1, 1000)
		.Select(i => (object)null));
	var SUT2 = SUT.ToArray();

	var sw = Stopwatch.StartNew();

	for (int i = 0; i < 5000; i++)
	{
		int count = 0;
		foreach (var item in SUT2)
		{
			count++;
		}
	}

	sw.Stop();
	Console.WriteLine($"TestForArray: {sw.Elapsed}");
}


private static void TestForListOfT()
{
	var SUT = new List<object>();
	SUT.AddRange(Enumerable.Range(1, 1000)
		.Select(i => (object)null));

	var sw = Stopwatch.StartNew();

	for (int j = 0; j < 5000; j++)
	{
		int count = 0;
		for (int i = 0; i < SUT.Count; i++)
		{
			var r = SUT[i];
			count++;
		}
	}

	sw.Stop();
	Console.WriteLine($"TestForListOfT: {sw.Elapsed}");
}

private static void TestForeachListOfT()
{
	var SUT = new List<object>();
	SUT.AddRange(Enumerable.Range(1, 1000)
		.Select(i => (object)null));

	var sw = Stopwatch.StartNew();

	for (int i = 0; i < 5000; i++)
	{
		int count = 0;
		foreach (var item in SUT)
		{
			count++;
		}
	}

	sw.Stop();
	Console.WriteLine($"TestForeachListOfT: {sw.Elapsed}");
}

private static void TestEnumeratorListOfT()
{
	var SUT = new List<object>();
	SUT.AddRange(Enumerable.Range(1, 1000)
		.Select(i => (object)null));

	var sw = Stopwatch.StartNew();

	for (int i = 0; i < 5000; i++)
	{
		int count = 0;
		var enumerator = SUT.GetEnumerator();

		while (enumerator.MoveNext())
		{
			var r = enumerator.Current;
			count++;
		}
	}

	sw.Stop();
	Console.WriteLine($"TestEnumeratorListOfT: {sw.Elapsed}");
}

@jeromelaban
Copy link
Contributor Author

jeromelaban commented Jul 26, 2021

I reran the sample above with e983168, and here are the results:

TestEnumeratorListOfT: 00:00:00.1430000
TestForeachListOfT: 00:00:00.3181000
TestForListOfT: 00:00:00.1434000
TestForArray: 00:00:00.0161000

Update:
Profiling of the TestForeachListOfT:
image

jeromelaban added a commit to jeromelaban/Wasm.Samples that referenced this issue Jul 26, 2021
@jeromelaban
Copy link
Contributor Author

This behavior was caused by a rogue MONO_INLINE_LIMIT=1 set for prior troubleshooting, which interferes with the ability of the AOT compiler to remove empty finally blocks.

Thanks @vargaz for the troubleshooting.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Codegen-AOT-mono
Projects
None yet
Development

No branches or pull requests

4 participants