Skip to content

Java/C++/C#: Improve performance of data flow with fields.#1799

Merged
hvitved merged 2 commits intogithub:masterfrom
aschackmull:java/fieldflow-perf
Aug 28, 2019
Merged

Java/C++/C#: Improve performance of data flow with fields.#1799
hvitved merged 2 commits intogithub:masterfrom
aschackmull:java/fieldflow-perf

Conversation

@aschackmull
Copy link
Copy Markdown
Contributor

The first pruning step is usually the performance bottleneck, so making that as fast as possible improves overall performance even if the later steps have to inspect a few more nodes.

@hvitved
Copy link
Copy Markdown
Contributor

hvitved commented Aug 22, 2019

Could we perhaps hold this back until #1761 is merged?

@hvitved
Copy link
Copy Markdown
Contributor

hvitved commented Aug 23, 2019

Could we perhaps hold this back until #1761 is merged?

It has been merged now, so can I ask you to rebase? Then I will run a dist-compare for C# over the weekend.

@aschackmull
Copy link
Copy Markdown
Contributor Author

Rebased.

@aschackmull
Copy link
Copy Markdown
Contributor Author

Btw. testing locally on jdk8 with a few different configurations this has been a clear consistent improvement in wall-clock time (using 8 threads), and the number of additional visited nodes have only gone up slightly in the first two pruning steps.

Copy link
Copy Markdown
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM. I would still like to run a dist-compare for C# before approving.

@hvitved
Copy link
Copy Markdown
Contributor

hvitved commented Aug 24, 2019

The dist-compare report for C# shows no change in performance for our benchmark projects. Happy to merge as this simplifies the code anyway.

@hvitved hvitved merged commit 853a3aa into github:master Aug 28, 2019
@aschackmull aschackmull deleted the java/fieldflow-perf branch August 29, 2019 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants