-
Notifications
You must be signed in to change notification settings - Fork 3
Some performance improvements #1
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
base: main
Are you sure you want to change the base?
Some performance improvements #1
Conversation
|
I'll look into these changes as soon as I have some time. But looks pretty good so far. Thanks! Do the tests pass? And do you have any data on the impact of the changes in terms of performance? |
|
I wanted to profile the Analyzer and wrote a test for it: It crashes with a StackOverflow:
Here are the file contents: |
|
Hi Robin,
I replied to your questions in my PR. Can you have a look at it? Some
performance improvements by p-26 · Pull Request #1 ·
hermanussen/ReferenceCopAnalyzer (github.com)
<#1>
Thanks,
Nicolas
Am Do., 7. Apr. 2022 um 11:23 Uhr schrieb Robin Hermanussen <
***@***.***>:
… I'll look into these changes as soon as I have some time. But looks pretty
good so far. Thanks!
Do the tests pass? And do you have any data on the impact of the changes
in terms of performance?
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD36L6JFU2XENA3O2FI2T4TVD2SSVANCNFSM5SYRLGNQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Hi Nicolas, Please be patient. I just haven't gotten to it yet. But I will eventually, I promise! |
Hi Robin, Do you know, roughly, when you'll have the opportunity to review my pull request? Thanks, |
|
Well, I took a look this weekend. I changed the unit test you added to include timings and tested with the current as well as your new code. These are my results (run in Current code: After applying your improvements: I wouldn't call that a significant performance increase. On the whole it seems rather slow anyway. Once I have some more time, I'll see if I can make some structural performance improvements. By the way; if you look at the huge difference between the slowest and fastest runs, it seems like the test is not fully isolated and Roslyn does some optimizations/caching. I think that needs to be addressed as well for testing. |



I implemented some performance improvements because the analyzer takes ~35s on my machine in our solution:
