Skip to content

[CompilerPerf] [WIP] trial: suppress needless uses of callvirt #2849

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

Closed
wants to merge 3 commits into from

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Apr 13, 2017

PR to reassess the perf impact of using call instead of callvirt in some situations. See context at #2844 and #468

@dsyme
Copy link
Contributor Author

dsyme commented Apr 13, 2017

Well, using this flag at least doesn't seem to hurt performance (we wouldn't expect it to though), see https://github.com/Microsoft/visualfsharp/blob/master/tests/scripts/compiler-perf-results.txt#L76

startup-to-parseonly parseonly-to-check   check-to-nooptimize  check-to-optimize    nooptimize-to-debug
10.75                28.57                42.86                56.34                52.50

v. some recent times on master:

10.12                30.98                45.96                61.91                51.16               
10.31                30.51                46.54                62.74                51.05               

That may even indicate an improvement, e.g. three of the phases seem to improve - but note that the times are pretty flaky - regular +/- 10% - so each are within the margin of error. I'll do more test runs.

@KevinRansom
Copy link
Contributor

@dsyme
What do you plan to do with this?

@dsyme
Copy link
Contributor Author

dsyme commented Apr 18, 2017

@KevinRansom I'm not sure yet. Let's leave it open and I'll get more perf data (the compiler perf scripts analyze the master branch plus any open PRs with [CompilerPerf] in the title)

@KevinRansom
Copy link
Contributor

@dsyme ... do you want to revisit this PR?

@KevinRansom
Copy link
Contributor

Actually looks like nothing much left of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants