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

Fix plan cache access. #2508

Closed
wants to merge 2 commits into from
Closed

Conversation

erichaugh
Copy link
Contributor

Overview

Fix performance regression is ShadowWrangler.methodInvoked().

Proposed Changes

Use containsKey() to check for entries in the Plan cache, instead of calling get() and checking for null. Otherwise, since CALL_REAL_CODE_PLAN is only ever set to null, calculatePlan() is called over and over again for methods whose Plan is CALL_REAL_CODE_PLAN, causing a ~7x slowdown in test run time.

There is probably some further clean up so CALL_REAL_CODE_PLAN is set to something non-null, but for now this fixes the performance regression.

…ling get() and checking for null. Otherwise, since CALL_REAL_CODE_PLAN is only ever set to null, calculatePlan() is called over and over again for methods whose Plan is CALL_REAL_CODE_PLAN, causing a ~7x slowdown in test run time.

There is probably some further clean up so CALL_REAL_CODE_PLAN is set to something non-null, but for now this fixes the performance regression.
@erichaugh erichaugh closed this Jun 13, 2016
@erichaugh erichaugh deleted the plan_cache branch June 13, 2016 15:26
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.

1 participant