Skip to content

Commit

Permalink
Add trace events to View#dispatchTouchEvent and RecyclerView#scrollStep
Browse files Browse the repository at this point in the history
In local testing, View#dispatchTouchEvent and RecyclerView#scrollStep
take up a large chunk of time under Choreographer callbacks and are
not currently captured in traces for the scroll jank dashboard.

Hopefully, adding these trace events will clarify at least some of
the jank currently attributed to "Choreographer(java_views=)"

RecyclerView#scrollStep is a package-private method in AndroidX that
isn't part of the View API, so required some minor changes to the
Bytecode rewriter to support methods missing from some class
hierarchies.

Bug: 1357024
Change-Id: I73f8a7c733501e69b744e3315bba9360819fe3a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3876627
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1044158}
NOKEYCHECK=True
GitOrigin-RevId: 85144e544dcecdc6928a86bc381ac08970953e2f
  • Loading branch information
Michael Thiessen authored and copybara-github committed Sep 7, 2022
1 parent 1f63704 commit 94dc620
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public void visitEnd() {
*/
private void writeOverrideCode(
MethodVisitor mv, final int access, final String name, final String descriptor) {
assert access != 0;
Type[] argTypes = Type.getArgumentTypes(descriptor);
Type returnType = Type.getReturnType(descriptor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PROTECTED;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ASM7;

import org.objectweb.asm.ClassVisitor;
Expand All @@ -19,7 +21,7 @@
* methods remain then we recurse on the class's superclass.
*/
class ParentMethodCheckerClassAdapter extends ClassVisitor {
private static final String OBJECT_CLASS_DESCRIPTOR = "java.lang.Object";
private static final String OBJECT_CLASS_DESCRIPTOR = "java/lang/Object";

private final ArrayList<MethodDescription> mMethodsToCheck;
private final ClassLoader mJarClassLoader;
Expand Down Expand Up @@ -62,8 +64,12 @@ public MethodVisitor visitMethod(
// This class contains methodToCheck.
boolean isMethodPrivate = (access & ACC_PRIVATE) == ACC_PRIVATE;
boolean isMethodFinal = (access & ACC_FINAL) == ACC_FINAL;
boolean isMethodPackagePrivate =
(access & (ACC_PUBLIC | ACC_PROTECTED | ACC_PRIVATE)) == 0;

// If the method is private or final then don't create an override.
methodToCheck.shouldCreateOverride = !isMethodPrivate && !isMethodFinal;
methodToCheck.shouldCreateOverride =
!isMethodPrivate && !isMethodFinal && !isMethodPackagePrivate;
}

return super.visitMethod(access, name, descriptor, signature, exceptions);
Expand All @@ -72,6 +78,15 @@ public MethodVisitor visitMethod(
@Override
public void visitEnd() {
if (mIsCheckingObjectClass) {
// We support tracing methods that are defined in classes that are derived from View,
// but are not defined in View itself. If we've reached the Object class in the
// hierarchy, it means the method doesn't exist in this hierarchy, so don't override it,
// and stop looking for it.
for (MethodDescription method : mMethodsToCheck) {
if (method.shouldCreateOverride == null) {
method.shouldCreateOverride = false;
}
}
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,12 @@ protected boolean shouldRewriteClass(String classPath) {
@Override
protected boolean shouldRewriteClass(ClassReader classReader) {
mMethodsToTrace = new ArrayList<>(Arrays.asList(
new MethodDescription(
"dispatchTouchEvent", "(Landroid/view/MotionEvent;)Z", Opcodes.ACC_PUBLIC),
new MethodDescription("draw", "(Landroid/graphics/Canvas;)V", Opcodes.ACC_PUBLIC),
new MethodDescription("onMeasure", "(II)V", Opcodes.ACC_PROTECTED),
new MethodDescription("onLayout", "(ZIIII)V", Opcodes.ACC_PROTECTED)));
new MethodDescription("onLayout", "(ZIIII)V", Opcodes.ACC_PROTECTED),
new MethodDescription("scrollStep", "(II[I)V", 0)));

// This adapter will modify mMethodsToTrace to indicate which methods already exist in the
// class and which ones need to be overridden. In case the class is not an Android view
Expand Down

0 comments on commit 94dc620

Please sign in to comment.