Fix explicit casts and improve tests.#18958
Conversation
| import java.lang.invoke.MethodHandles.Lookup; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.IdentityHashMap; |
|
Cool! I like the idea also to randomize the caching, although I am not so happy with the initialCallSiteDepth parameter passed into every invokeDynamic call. I agree this is the best solution, but I have to think about it. Do you know if this has an impact on speed of invokedynamic? |
No, as this phase's job is just to return a CallSite which is permanent? We already pass args here, and they hint that the call is going to happen via invokeWithArguments if you have any at all. Also i ran benchmarks and they are happy. For now at least, we need the test coverage, for the math operators. We can test it directly some other way and then remove it. But these slave labor tests are exhausting man. |
|
If we arent going to pass this param, I'll open a separate PR to remove dynamic math operator support and just let math be slower (boxing and instanceof like before) but correct. Otherwise, if we want the performance improvement that comes with that math operator support, we need to ensure its tested, and that is what this PR is about. |
|
I am fine. I just tried to think about some better way to do it. But I did not find one. |
|
I saw the benefit when i saw minor inconsistencies like WrongMethodType vs ClassCastException for operators depending on how many types we had seen. We can of course replace this with explicit tests, but currently we need all the tests we can get because it is not straightforward to see the logic. In testing all this, i found a bug in EChain with compound assignment that only impacts shift operators, for example. |
|
+1 from me. |
|
LGTM |
math operators for dynamic types adopt the return value, but we don't handle explicit user casts.
for the cached case, we can use explicitCastArguments (but with checks to prevent the insane stuff it will do with boolean types). For the fallback case we use a slower method.
I beefed up testing around this area and found a few bugs here and there.