On Wed, 11 Jan 2023 18:44:20 GMT, Archie L. Cobbs <d...@openjdk.org> wrote:
>> Also, looking at the loop test more closely, it seems to me that what the
>> compiler needs to do is to prove that there can be possible paths by which a
>> `this` can land into ref4.
>>
>> If we build a graph of all the assignments, we get:
>>
>> ref4 <- ref3 <- ref2 <- ref1 <- this
>>
>> So, if we ask "can ref4 possibly contain `this`?" we could "walk" the
>> variable dependencies backwards and discover that, yes, there exist a
>> possible path in which `this` would get there.
>>
>> Now, of course without a loop this can never be a real issue (since you can
>> never send a `this` fully down the chain) - but again, this is a question of
>> how much effort should be spend to handle false negatives in what look like
>> a pathological case.
>
> Good idea. Looks like the difference is in the noise, at least on my Macbook:
>
> Builds of master (jdk-21+3-69-gc6588d5bb3f)
> ==================================
>
> Build times:
>
> real 2m24.650s
> user 13m46.727s
> sys 2m33.554s
>
> real 2m27.224s
> user 13m43.464s
> sys 2m37.251s
>
> real 2m26.658s
> user 13m42.578s
> sys 2m36.133s
>
> Builds of ThisEscape (jdk-21+3-125-g6e96a7d76f8)
> ==================================
>
> Modifications:
>
> - Reverted files in the make/ subdirectory to enable warning
> - Commented out lines 363-382 in ThisEscapeAnalyzer.java
> so no warnings are actually reported
>
> Build times:
>
> real 2m25.912s
> user 13m45.860s
> sys 2m32.741s
>
> real 2m27.213s
> user 13m44.830s
> sys 2m36.596s
>
> real 2m25.756s
> user 13m42.889s
> sys 2m35.659s
Regarding the assignment graph approach, I think that would work if the
references are bouncing around strictly between variables, but what if the
chain includes any of the more complicated stuff that is currently being
tracked, such as various Java expressions, method invocations, conditionals,
etc.?
Consider this example:
public class ThisEscapeLoop {
public ThisEscapeLoop() {
ThisEscapeLoop ref1 = this;
ThisEscapeLoop ref2 = null;
ThisEscapeLoop ref3 = null;
ThisEscapeLoop ref4 = null;
for (int i = 0; i < 100; i++) {
ref4 = this.returnMe(ref3);
ref3 = ref2;
ref2 = ref1;
if (ref4 != null)
ref4.mightLeak();
}
}
public <T> T returnMe(T x) {
return x;
}
public void mightLeak() {
}
}
If you are only looking at variable assignments with values that are other
variables, then the chain "breaks" when `ref4` is assigned indirectly via
`returnMe()`, and you miss the leak.
-------------
PR: https://git.openjdk.org/jdk/pull/11874
