hasGlyph performance [170587912]

Fixed

Bug

Status Update

No update yet.

Description

si...@google.com

created issue #1

Oct 12, 2020 02:06AM

We had reports that EmojiProcessor.hasGlyph is rather heavy on API 23+ despite the cache where each emoji is checked for rendering only once for each EmojiCompat instance.

Investigate the culprit.

Comments

si...@google.com <si...@google.com> #2Oct 12, 2020 02:24AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

https://android-review.googlesource.com/1123258

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

no...@google.com <no...@google.com> #3Oct 12, 2020 02:40AM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

si...@google.com <si...@google.com> #4Oct 12, 2020 05:47AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

Re-factor how errors are dismissed when query is re-written

This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (

b/140759491

).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.

This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.

Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

https://android-review.googlesource.com/1288456

ap...@google.com <ap...@google.com> #5Oct 13, 2020 05:03PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 1b3056f484ecb3b82e300362d42327dce145205a
Author: Siyamed Sinir <siyamed@google.com>
Date: Sun Oct 11 19:25:27 2020

Expose GlyphChecker interface

EmojiCompat.Config accepts GlyphChecker in the API.

Test: Added new tests
Test: ./gradlew emoji:test
Test: ./gradlew emoji:connectedAndroidTest

Relnote: "Added new API `EmojiCompat.Config#setGlyphChecker` that enables developers
to provide custom emoji glyph check behavior."

Bug: 170587912
Change-Id: Ibc95e158765d392cb8726e53b0f8cac7961f0373

M emoji/core/api/current.txt
M emoji/core/api/public_plus_experimental_current.txt
M emoji/core/api/restricted_current.txt
M emoji/core/src/androidTest/java/androidx/emoji/text/ConfigTest.java
M emoji/core/src/androidTest/java/androidx/emoji/text/EmojiCompatTest.java
M emoji/core/src/androidTest/java/androidx/emoji/text/TestConfigBuilder.java
M emoji/core/src/main/java/androidx/emoji/text/EmojiCompat.java
M emoji/core/src/main/java/androidx/emoji/text/EmojiProcessor.java

https://android-review.googlesource.com/1456995

si...@google.com <si...@google.com> Oct 13, 2020 05:04PM

Marked as fixed.

se...@google.com <se...@google.com> #6Jul 22, 2021 05:01PM

Leaving closed, but listing another option for developers reading this thread:

hasGlyph is cached in emojicompat per glyph, so one option would be to process a string containing all emoji in a background thread. This would effectively avoid any calls to hasGlyph to render user-defined strings later.

Issue 170587912