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

Optimize static method invocation with rvalue target expression #62

Closed
aunkrig opened this issue Oct 17, 2018 · 5 comments
Closed

Optimize static method invocation with rvalue target expression #62

aunkrig opened this issue Oct 17, 2018 · 5 comments

Comments

@aunkrig
Copy link
Member

aunkrig commented Oct 17, 2018

https://github.com/apache/drill/branches/1.14.0/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/AloadPopRemover.java
correctly reports that Janino doesn't optimize this case:

localVariable.staticMethod();

The code generated by Janino retrieves the local variable value and immediately discards it.

@aunkrig
Copy link
Member Author

aunkrig commented Oct 17, 2018

Janino now checks if the target expression has side effects, and if not, completely eliminates the code.
UnitCompiler.compileGet2(MethodInvocation mi).

@vvysotskyi
Copy link
Contributor

@aunkrig, thanks for fixing this issue!
I tried to reproduce it in Drill, but all tests are passed when this workaround is removed, so I can't verify it.

@aunkrig
Copy link
Member Author

aunkrig commented Oct 24, 2018

I tried to reproduce it in Drill, but all tests are passed when this workaround is removed, so I can't verify it.

I suppose that means "problem resolved"!?

@vvysotskyi
Copy link
Contributor

Yes, that means "problem resolved", but perhaps in one of the previous releases. Thanks!

@aunkrig aunkrig closed this as completed Oct 24, 2018
@aunkrig
Copy link
Member Author

aunkrig commented Nov 13, 2018

Version 3.0.11 is out the door, and includes the fix for this issue.

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

No branches or pull requests

2 participants