Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace emoji parser library #1442

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

maniac103
Copy link
Collaborator

emoji-java is unmaintained since 2019, so replace it with a library that receives updates.

@maniac103
Copy link
Collaborator Author

@Fs00 Please give this a try as well.

@Fs00
Copy link
Contributor

Fs00 commented Feb 10, 2025

Were you following the development of that library too? 😄
I'll give this PR a whirl once it's rebased.

Don't forget to update the used libraries in the Readme and in About screen of the app 😉

@maniac103
Copy link
Collaborator Author

Rebase done.

Don't forget to update the used libraries in the Readme and in About screen of the app 😉

Good point, thanks.

emoji-java is unmaintained since 2019, so replace it with a library that
receives updates.
@Fs00
Copy link
Contributor

Fs00 commented Feb 11, 2025

I've played a bit with it and the emoji parsing behaves more faithfully to the GitHub website, which is a great plus.
Unfortunately, there's a problem: when opening for the first time a screen that uses EmojiManager.replaceAliases, a very noticeable freeze occurs. On my Xperia X Compact it takes ~2 seconds (on a debug build though, it's probably a bit less bad on a release build).
The culprit appears to be the static constructor of EmojiManager, which precomputes several maps from the emoji list. I doubt that all of them are relevant to our use case.
emoji_flamegraph
(here you see that the constructor only takes 180ms because this flamegraph was taken on the AS emulator on my PC, which is way more powerful than the phones I own)

I'd suggest checking with the library author if something can be done on their side to improve the situation.
Did you notice this while testing?

@maniac103
Copy link
Collaborator Author

Did you notice this while testing?

So far, I just tested on the emulator. I now gave this a try on my phone (Pixel 8), and it looks fine there as well, taking 200ms ... but admittedly the phone is quite a bit faster than Xperia X Compact.

Let's see what the author's response to this is: felldo/JEmoji#57

@maniac103
Copy link
Collaborator Author

maniac103 commented Feb 13, 2025

@Fs00 What does this diff do to the loading times on your device?

diff --git a/app/src/main/java/com/gh4a/Gh4Application.java b/app/src/main/java/com/gh4a/Gh4Application.java
index 1f2738e8..0ab4a27c 100644
--- a/app/src/main/java/com/gh4a/Gh4Application.java
+++ b/app/src/main/java/com/gh4a/Gh4Application.java
@@ -29,6 +29,8 @@ import com.gh4a.worker.NotificationsWorker;
 import com.meisolsson.githubsdk.model.User;
 import com.tspoon.traceur.Traceur;
·
+import net.fellbaum.jemoji.EmojiManager;
+
 import org.ocpsoft.prettytime.PrettyTime;
·
 import java.util.HashSet;
@@ -67,6 +69,7 @@ public class Gh4Application extends Application implements
         super.onCreate();
·
         sInstance = this;
+        new Thread(() -> EmojiManager.getEmoji("")).start();
·
         SharedPreferences prefs = getPrefs();
·

BTW, API 24 emulator seems quite a bit slower than API 34 emulator, so the issue likely is in the optimization of the static initializer.

@maniac103
Copy link
Collaborator Author

@Fs00 And for another test, please try whether updating to 1.7.1 (instead of the thread approach) helps.

@Fs00
Copy link
Contributor

Fs00 commented Feb 15, 2025

1.7.1 improves loading times significantly, but not enough not to be noticeable on older devices.
The improvements I've measured (on debug builds, with a fair amount of approximation) are the following:

  • Xperia X Compact: ~2 secs down to 0,8-1 sec
  • Samsung Galaxy A3 2017: ~3-3,5 secs down to 1,5 sec
  • AS emulator on my PC (which I've taken the above flamegraph from): 180ms down to 33ms

However, replacing the line you suggested above with the following one:

new Thread(() -> EmojiManager.replaceAliases("", (p1, p2) -> "")).start();

pretty much prevents freezes to occur even on old devices. This helps even when the app starts directly on a screen that uses replaceEmojiAliases (e.g. the repository activity), because initialization is now fast enough to complete before the data is loaded and displayed from what I've seen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants