Regression after removal of field lookup cache [265148324]

Fixed

Bug

Status Update

No update yet.

Description

ze...@google.com

created issue #1

Jan 11, 2023 03:59PM

The resolution of b/151804585 did not introduce a new field backing, thus lookups within a class are linear search in the field arrays. This has noticeable performance impact witnessed in b/264430795

Using the dump information provided in b/264430795, the initial regression came with CLs:

353672ms 78a3b2fe030b465c46c1b2f858cada93659a0e40 Minor cleanup prior to definitionFor(DexField) removal
335307ms 80ef5bef88b866f9dea82efc73b024ee751b42e2 Remove all uses of definitionFor(DexMember)
337779ms e7186a7da0d3edc84b389169614c60be4e208b14 Remove all uses of definitionFor(DexReference)
267364ms 225ddc66dc42ce0fdef363d17b61c88807b661ed Add CLI tests for various flag additions and fixes.

With the bottom CL being fairly equal in runtime to the original report at version 2.0.99.

Total regression is about ~1.5min on a previously 4.5min build (increase of about 33% in compiletime).

We should introduce a (potentially) hash-map based field collection backing mirroring the backing for methods.

Comments

ze...@google.com <ze...@google.com> #2Jan 11, 2023 04:20PM

I also have the same issue so it seems to be a real problem.

ap...@google.com <ap...@google.com> #3Jan 13, 2023 12:02PM

Dup here:

http://code.google.com/p/android/issues/detail?id=27959&can=1&q=fixed%20position&colspec=ID%20Type%20Status%20Owner%20Summary%20Stars

Though it's a bit more detailed with the minimum necessary to reproduce.

ap...@google.com <ap...@google.com> #4Jan 16, 2023 11:14AM

why is this marked has Obsolete? this is still a bug that is happening and its difficult to fix.
for all who are looking for a solution use this:

http://stackoverflow.com/questions/16200972/android-css-positionfixed-after-a-device-rotate

ap...@google.com <ap...@google.com> #5Jan 16, 2023 11:14AM

Project: r8
Branch: 8.0

commit 9f5d05983b53811f73c2854e7bf8c928e14daf37
Author: Ian Zerny <zerny@google.com>
Date: Mon Jan 16 10:43:08 2023

Introduce a field collection with array and map-based backings.

Bug:

b/265148324

Change-Id: I5155084088ce04e389073314d763b4a952610add

M src/main/java/com/android/tools/r8/graph/DexClass.java
M src/main/java/com/android/tools/r8/graph/DexClasspathClass.java
A src/main/java/com/android/tools/r8/graph/DexFieldSignature.java
M src/main/java/com/android/tools/r8/graph/DexProgramClass.java
A src/main/java/com/android/tools/r8/graph/FieldArrayBacking.java
A src/main/java/com/android/tools/r8/graph/FieldCollection.java
A src/main/java/com/android/tools/r8/graph/FieldCollectionBacking.java
A src/main/java/com/android/tools/r8/graph/FieldMapBacking.java
M src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java

https://r8-review.googlesource.com/75154

ze...@google.com <ze...@google.com> Jan 18, 2023 08:37AM

Marked as fixed.

Issue 265148324