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] recursive foreach loop is 17x slower than for loop over List<T> using profiled AOT #48179

Closed
jeromelaban opened this issue Feb 11, 2021 · 8 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-Codegen-AOT-mono
Milestone

Comments

@jeromelaban
Copy link
Contributor

Description

full repro is coming

Considering the following cpde:

static void Bench(List<int> array)
{
	for (int i = 0; i < innerCount; i++)
	{
		Nest(0);
	}

	void Nest(int depth)
	{
		if(depth == maxDepth)
		{
			return;
		}

		for (int i = 0; i < array.Count; i++)
		{
			Nest(depth + 1);
		}
	}
}

Changing the loop type from for to foreach results in a significant performance degradation:

profiled AOT Nested loops:

  • for loop 00:00:00.0008850
  • foreach loop 00:00:00.0138100

Full-AOT Nested loops:

  • for loop: 00:00:00.0002350
  • foreach loop: 00:00:00.0110250

Configuration

bb65067

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 Feb 11, 2021
@kg kg added the arch-wasm WebAssembly architecture label Feb 11, 2021
@ghost
Copy link

ghost commented Feb 11, 2021

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

Issue Details

Description

full repro is coming

Considering the following cpde:

static void Bench(List<int> array)
{
	for (int i = 0; i < innerCount; i++)
	{
		Nest(0);
	}

	void Nest(int depth)
	{
		if(depth == maxDepth)
		{
			return;
		}

		for (int i = 0; i < array.Count; i++)
		{
			Nest(depth + 1);
		}
	}
}

Changing the loop type from for to foreach results in a significant performance degradation:

profiled AOT Nested loops:

  • for loop 00:00:00.0008850
  • foreach loop 00:00:00.0138100

Full-AOT Nested loops:

  • for loop: 00:00:00.0002350
  • foreach loop: 00:00:00.0110250

Configuration

bb65067

Regression?

No

Other information

Author: jeromelaban
Assignees: -
Labels:

arch-wasm, untriaged

Milestone: -

jeromelaban added a commit to jeromelaban/Wasm.Samples that referenced this issue Feb 11, 2021
@lewing lewing added area-Codegen-AOT-mono and removed untriaged New issue has not been triaged by the area owner labels Feb 11, 2021
@SamMonoRT SamMonoRT added this to the 6.0.0 milestone Jun 24, 2021
@SamMonoRT
Copy link
Member

@kg @vargaz do we think we get to this prior to Preview 7 ?

@kg
Copy link
Contributor

kg commented Jun 24, 2021

I don't have enough context to answer that, zoltan would

@vargaz
Copy link
Contributor

vargaz commented Jun 24, 2021

@jeromelaban Does this still happen ?

@jeromelaban
Copy link
Contributor Author

@vargaz unless #54272 is fixed, I think this still applies.

@SamMonoRT
Copy link
Member

@jeromelaban - can we close this, seems like 54272 was closed.

@jeromelaban
Copy link
Contributor Author

@SamMonoRT Indeed we can.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 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

5 participants