Skip to content

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.

@maniac103
Copy link
Collaborator Author

@Fs00 What about 1.7.2 without the thread hack?

@Fs00
Copy link
Contributor

Fs00 commented Apr 3, 2025

I've just tested 1.7.2 but it crashes:

java.io.InvalidClassException: net.fellbaum.jemoji.Emoji; local class incompatible: stream classdesc serialVersionUID = 7671752869581901954, local class serialVersionUID = 7835010533519095060
  at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:652)
  at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1743)
  at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1624)
  at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1902)
  at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1442)
  at java.io.ObjectInputStream.readObject(ObjectInputStream.java:430)
  at java.util.HashMap.readObject(HashMap.java:1558)
  at java.lang.reflect.Method.invoke(Native Method)
  at java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1109)
  at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2085)
  at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1935)
  at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1442)
  at java.io.ObjectInputStream.readObject(ObjectInputStream.java:430)
  at net.fellbaum.jemoji.EmojiManager$InitHelper.readFileAsObject(EmojiManager.java:68)

@felldo
Copy link

felldo commented Apr 3, 2025

Hi @Fs00 , I am the author of the library. I have no idea why this is happening. All tests passed and when using the library in a Java 17 project it also works flawless (the newly released version on maven central). Maybe this is an android related issue again?
Anyway, is it possible for you to quickly include a local library instead of the version available on maven central and see if this fixes the issue? That way I don't have to release another potential broken version.

jemoji-1.7.3.jar.zip

@Fs00
Copy link
Contributor

Fs00 commented Apr 4, 2025

Hello @felldo, I've tested the attached JAR and it didn't crash anymore, but loading times are massively slower than v1.7.1.
For comparison, on the aforementioned Samsung Galaxy A3 2017 initialization now takes 9 secs compared to the 1,5 secs of v1.7.1.

@felldo
Copy link

felldo commented Apr 4, 2025

Hello @felldo, I've tested the attached JAR and it didn't crash anymore, but loading times are massively slower than v1.7.1. For comparison, on the aforementioned Samsung Galaxy A3 2017 initialization now takes 9 secs compared to the 1,5 secs of v1.7.1.

Very weird problem which I don't know why it happens. Now it should deserialize a java object instead of initializing 100+ interfaces which I suspected to be the problem (Though a bit of a performance degradation is exptected in comparison to emoji-java, as JEmoji is far superior in terms of available data for emojis and including the newest emojis).
Can you profile the application to find out what line in the library is causing this long delay?

@felldo
Copy link

felldo commented Apr 4, 2025

So, I found some old phones and could test the library myself and do a bit of research. I got these results:

Samsung Galaxy S7 Edge

  • Library version 1.7.2: 2700ms
  • Improved code: 360ms

Samsung Galaxy S4:

  • Library version 1.7.2: 16300ms
  • Improved code: 3300ms

There are now some significant improvements by simply using a BufferedInpuStream. So the used InputStream to read the emoji file seems to be the bottleneck. I guess we are now back to the "original" improved time of 1.5s on your Samsung Galaxy A3 2017 or maybe even a bit better. If you want to test it with your phone again, I attached the improved library code below, otherwise I am going to publish a new version soon.

jemoji-1.7.3.jar.zip

@Fs00
Copy link
Contributor

Fs00 commented Apr 5, 2025

Can confirm that the new build improves things a lot, loading performance is now the same as 1.7.1.

Since we've seen that preloading emojis upfront is required to avoid stutters on older phones, could the library expose a few functions for explicitly initializing the required emoji data? So that we could call something like EmojiManager.preloadEmojiAliases() instead of EmojiManager.replaceAliases("", (p1, p2) -> "") to achieve the same thing in a less cryptic way 🙂

@felldo
Copy link

felldo commented Apr 5, 2025

The "slow" part of the library (reading the emoji data) is triggered by calling any method in the EmojiManager.
After that there are 2 init methods: for unicode (basic emoji methods ) and alias related methods. The alias related initialisation is a bit faster because less compuation is going to be performed after reading the emoji data. Something less cryptic would be calling getByAlias(String) or extractAliases(String) (both can be empty string/null).

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.

3 participants