matrix-org / emojibase-bindings

Packages emojibase bindings for iOS and Android
Apache License 2.0
3 stars 1 forks source link

Kotlin bindings, tests, swift/kotlin CI validation and release script #2

Closed langleyd closed 1 year ago

langleyd commented 1 year ago
langleyd commented 1 year ago

I just added Jorge as a reviewer, not sure why, it requested again from you.

jmartinesp commented 1 year ago

Could we measure how long it takes for load the data from the JSON file in the tests?

t3chguy commented 1 year ago

@jmartinesp could replace the JSON with some form of codegen, like we do for https://github.com/matrix-org/matrix-analytics-events

jmartinesp commented 1 year ago

@jmartinesp could replace the JSON with some form of codegen, like we do for matrix-org/matrix-analytics-events

That would be great since this parsing is done when the app boots and could potentially affect startup time, although we're using an initializer which should make it happen on background. However, I'm not sure how complex that could be and I don't want to block this release for a long time.

langleyd commented 1 year ago

@jmartinesp That could be a lot of code/symbols generated in this case? Not to say we shouldn't just something to be weighed up?

I suppose lets see how long it takes and I'll report back. On the iOS side it was loading json in the last version so was a like for like.

t3chguy commented 1 year ago

Could we measure how long it takes for load the data from the JSON file in the tests?

Not reasonably, the free Github Actions runners are massively variable in their performance, we saw an insane variance in attempted performance testing for Element Web in GHA.

jmartinesp commented 1 year ago

@jmartinesp That could be a lot of code/symbols generated in this case? Not to say we shouldn't just something to be weighed up?

I think that's why they're using a chunk strategy + using static variables, so they're only loaded when needed: https://github.com/vanniktech/Emoji/blob/master/emoji-twitter/src/commonMain/kotlin/com/vanniktech/emoji/twitter/category/FlagsCategory.kt#L32

langleyd commented 1 year ago

On my iPhone 12 it takes between 0.09 and 0.13 seconds to load.

On EIX the current implementation, the emoji data is tied to the user session, so it stays in memory vs being loaded on demand when the user loads the picker.

jmartinesp commented 1 year ago

I run the instrumentation tests too with some measurements on a 5 year old phone and it took ~140ms, which seems reasonable.

In fact, I tested it properly with several iterations and it takes ~35ms, which is a lot better than expected.