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

Don't use once read symbol heuristics in UseDef at warm #6093

Merged
merged 1 commit into from Jul 7, 2021

Conversation

gita-omr
Copy link
Contributor

@gita-omr gita-omr commented Jul 5, 2021

Don't use once read symbol heuristics in UseDef at warm

- Use onceReadSymbolsIndices at < warm since it helps performance
  without considerably increasing compile time

@vijaysun-omr vijaysun-omr self-assigned this Jul 5, 2021
@vijaysun-omr
Copy link
Contributor

Jenkins build all

@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 5, 2021

Made WIP since I still need to run through personal build..

@vijaysun-omr
Copy link
Contributor

Jenkins build all

@fjeremic
Copy link
Contributor

fjeremic commented Jul 5, 2021

Could we improve the commit message with a reason why this change is being made, or at the very least add a comment to this PR explaining things? I can see someone looking up the history of this change in the future and wondering what drove us to make this change. Also for PRs with one commit we should try to keep the commit title the same as the PR title (+ sentence case capitalization of first work for consistency with other commits).

    - Use onceReadSymbolsIndices at < warm since it helps performance
      without considerably increasing compile time
@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 7, 2021

Could we improve the commit message with a reason why this change is being made, or at the very least add a comment to this PR explaining things? I can see someone looking up the history of this change in the future and wondering what drove us to make this change. Also for PRs with one commit we should try to keep the commit title the same as the PR title (+ sentence case capitalization of first work for consistency with other commits).

I can also see that :) Fixed.

@vijaysun-omr
Copy link
Contributor

Jenkins build all

@gita-omr gita-omr changed the title Don't use once read symbol heuristics in UseDef at warm WIP: Don't use once read symbol heuristics in UseDef at warm Jul 7, 2021
@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 7, 2021

Please don't merge yet. I found a personal build failure that I need to debug.

@gita-omr gita-omr changed the title WIP: Don't use once read symbol heuristics in UseDef at warm Don't use once read symbol heuristics in UseDef at warm Jul 7, 2021
@gita-omr
Copy link
Contributor Author

gita-omr commented Jul 7, 2021

I think it was unrelated failure. Please merge if there are no other issues.

@vijaysun-omr
Copy link
Contributor

vijaysun-omr commented Jul 7, 2021

Heuristic change that looks fine to me and is backed up by data. Checks have passed. Merging.

@vijaysun-omr vijaysun-omr merged commit 5567b92 into eclipse:master Jul 7, 2021
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.

None yet

3 participants