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

Microbenchmark System.Numerics.Tests.Perf_BitOperations.PopCount_ulongis 464 % slower on Wasm #41454

Closed
Tracked by #47244
naricc opened this issue Aug 27, 2020 · 20 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-System.Numerics tenet-performance Performance related issue
Milestone

Comments

@naricc
Copy link
Contributor

naricc commented Aug 27, 2020

Microbenchmark System.Numerics.Tests.Perf_BitOperations.PopCount_ulongis 464 % slower on Wasm vs. desktop interpreter.

Repro Instructions for running benchmarks locally

Results page when reported.

Automated runs

@naricc naricc added arch-wasm WebAssembly architecture tenet-performance Performance related issue labels Aug 27, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Numerics untriaged New issue has not been triaged by the area owner labels Aug 27, 2020
@ghost
Copy link

ghost commented Aug 27, 2020

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

@EgorBo
Copy link
Member

EgorBo commented Aug 27, 2020

ah, can fix as part of #40069

@SamMonoRT SamMonoRT added this to the 6.0.0 milestone Aug 27, 2020
@marek-safar marek-safar removed the untriaged New issue has not been triaged by the area owner label Aug 31, 2020
@SamMonoRT
Copy link
Member

@EgorBo - was this fixed as part of #40069 If so, please close this issue.

@naricc
Copy link
Contributor Author

naricc commented Oct 23, 2020

Looking at the benchmark results for today an the last several days, it seems like this benchmark is now around 280 % slower on Wasm vs. Desktop interpreter. That is probably an acceptable slowdown for wasm.

Latest results . I don't know how to link to just this benchmark, so that will load all of them, which will take a while.

@naricc
Copy link
Contributor Author

naricc commented Nov 4, 2020

@EgorBo I think we can close this out unless there is any follow up that need to be done?

@EgorBo
Copy link
Member

EgorBo commented Nov 4, 2020

@SamMonoRT @naricc #40069 wasn't merged so that must be something else I guess.

@SamMonoRT
Copy link
Member

Let's keep an eye on the results of this benchmark in next 3-4 days, and then close if we determine it is improved. @EgorBo do you think you changes might help make this even more performant ?

@EgorBo EgorBo removed their assignment Feb 10, 2021
@tannergooding
Copy link
Member

@naricc, is there any update here. Is this something that is going to be worked on for 6.0.0?

If not, I'd like to either move this to future or close, as appropriate.

@naricc
Copy link
Contributor Author

naricc commented Jun 18, 2021

@tannergooding This is not being actively worked on for 6.0.0; I am having some issues verifying weather or not is resolved right now. I will move it to future, and then close it when I am able to get the data (if merited).

@naricc naricc modified the milestones: 6.0.0, Future Jun 18, 2021
@SamMonoRT SamMonoRT modified the milestones: Future, 7.0.0 Jun 21, 2021
@ilonatommy
Copy link
Member

@naricc is there anything new about this? Are we able to ship it with 7.0?

@naricc
Copy link
Contributor Author

naricc commented Jun 20, 2022

@lewing lewing modified the milestones: 7.0.0, 8.0.0 Jul 25, 2022
@lewing
Copy link
Member

lewing commented Jul 25, 2022

@SamMonoRT lets triage this properly when 8 starts

@kotlarmilos kotlarmilos assigned kotlarmilos and unassigned naricc Feb 28, 2023
@lewing
Copy link
Member

lewing commented Jul 11, 2023

@kotlarmilos is there a status update on this?

@kotlarmilos
Copy link
Member

The BitOperations have been intrinsified on both platforms in #82827. The microbenchmark System.Numerics.Tests.Perf_BitOperations.PopCount_ulongis is about 60% slower on the Wasm than on Interpreter.

Is it expected to have decreased performance when running on Wasm?

@lewing
Copy link
Member

lewing commented Jul 17, 2023

WasmBase was being incorrectly disabled and that was fixed in #88523 lets make sure we have accurate numbers for this now

@radekdoulik
Copy link
Member

radekdoulik commented Jul 17, 2023

@kotlarmilos what is the configuration with 464% perf. Is it aot or interpreter on wasm?

edit: or actually these 60% you measured.

@lewing
Copy link
Member

lewing commented Jul 17, 2023

@kg is there a jiterpreter intrinsic for i64.popcnt ?

@lewing
Copy link
Member

lewing commented Jul 17, 2023

@kg is there a jiterpreter intrinsic for i64.popcnt ?

I see there is

@kotlarmilos
Copy link
Member

@kotlarmilos what is the configuration with 464% perf. Is it aot or interpreter on wasm?

@radekdoulik Sorry for the late reply, it is wasm interpreter.

@lewing
Copy link
Member

lewing commented Jul 21, 2023

We looked into this and consider it closed

@lewing lewing closed this as completed Jul 21, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Numerics tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests