Status Update
Comments
lo...@gmail.com <lo...@gmail.com> #2
Could you look into it @christofferqa?
The issue is with this class:
class MySpi : Spi {
@Volatile
private var myApi: Api? = null
// Incorrectly rewritten to return null;
override fun createApi(): Api {
return myApi ?: synchronized(this) {
myApi ?: MyApi().also { myApi = it }
}
}
}
The error seems to be that the ArgumentPropagator believes that the fieldState of myApi is ExactDynamicType(@NotNull MyApi)
while it should be ExactDynamicType(@Nullable MyApi)
It then removes the first branch (assuming myApi is not null):
return myApi ?: synchronized(this) {
myApi ?: MyApi().also { myApi = it }
}
is rewritten into:
return myApi
And then it eventually concludes that myApi is always null, replacing the code by
return null
The ArgumentPropagatorCodeScanner analyzes the assignment myApi = it
and correctly set join the state to ExactDynamicType(@NotNull MyApi)
However the ArgumentPropagator fails to understand that myApi
can be read before being written in the method, relying on the default null value, so it fails to join null
to the dynamic type.
ma...@google.com <ma...@google.com>
ap...@google.com <ap...@google.com> #3
Yes I can take a look. Would it be possible for you to add a regression test of this issue?
na...@google.com <na...@google.com> #4
Project: r8
Branch: main
Author: Christoffer Adamsen <
Link:
Reproduce field propagation issue with ServiceLoader
Expand for full commit details
Reproduce field propagation issue with ServiceLoader
Bug: b/389737060
Change-Id: Id850448f68b999194bc0380dfca625f4f68bc31b
Files:
- A
src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/DefaultFieldValueJoinerWithServiceLoaderTest.java
Hash: 6ba9af34b15f2a0d3fe356f6e7a5a4071128aed9
Date: Wed Feb 12 09:31:01 2025
Description
Component used: Jetpack macrobenchmark Version used: 1.4.0-alpha07 Devices/Android versions reproduced on: Pixel 4 XL Android 14 aosp
Sometimes there are resynced frames when frame is prepared for too long: "Choreographer#doFrame - resynced to 8643135 in 21,7ms" and there is logic added here to filter these out in FrameTimingQuery:
But there's a check later that checks if all slices have frameId:
Frame id is number that comes after "Choreographer#doFrame" And it checks for frameId in unfiltered slices where
Choreographer#doFrame - resynced to 8643135 in 21,7ms
is present. It tries to convert"-"
to id, fails to do so and returns null.It results in test failing because of that check.
Should be easy fix - just filter first and check for frameIds presence in filtered traces