wordpress-mobile / AztecEditor-Android

A reusable native Android rich text editor component.
Mozilla Public License 2.0
684 stars 115 forks source link

Look into ways to improve the performance of `fromHtml` #800

Open cassgenerator opened 5 years ago

cassgenerator commented 5 years ago

While looking into the root causes of this issue logged against Guttenberg Mobile. I tracked down that one of the causes of the lag that is the call to AztectTexts fromHtml call.

I made some small modifications to the app to load a large post from this comment into a string then try to load it. Then I profiled it loading that into the TextView.

From the Flame Chart we can see that the fromHtml call itself is accounting for a large portion of the time to load this string into the view, and there is also a large portion for the setText call on the TextView. Both expected results Screen Shot 2019-03-28 at 5 19 41 pm

However if we switch over to the Call Chart there is something very interesting happening in the second half of the chart (where the view is still unresponsive despite showing the new text). We can see that `refreshText is being called at least 8 times. Which to me looks like an opportunity for performance gains. Screen Shot 2019-03-28 at 5 19 30 pm

I think there are a number of items to look into, please note I don't expect these to all bear fruit but they are worth noting.

Expected

When fromHtml is called on AztecText the main thread should be blocked for smallest amount of time possible.

Observed

We are parsing and applying styling to the string in the main thread, then calling setText on the underlying TextView multiple times before the user is able to interact with the application again.

Reproduced

  1. Download the post html from the comment mentioned above, save this as a text file into the raw resource directory
  2. Add a button or other way to trigger the trigger our test code. In the test code do the following
  3. Load the file into a string (I've included a snippet of the code I used below)
  4. Call aztec.visualEditor.fromHtml(largeString)

Tested

Android Emulator on Android 9 with develop at the point of writing this issue 14b9f948b57657ab1613aefea5e9d366a436f180

Stream conversion code

@Throws(Exception::class)
fun convertStreamToString(inputStream: InputStream): String {
    val reader = BufferedReader(InputStreamReader(inputStream))
    val builder = StringBuilder()

    var line: String? = null
    line = reader.readLine()
    while (line != null) {
        builder.append(line).append('\n')
        line = reader.readLine()
    }
    reader.close()
    return builder.toString()
}
cassgenerator commented 5 years ago

I got some more time today to look into this. The multiple calls to setText seem to be due to the multiple images in the HTML being loaded and setText getting called each time one is ready for display.

I stumbled on this article regarding spans. It looks like if we store a reference to the internally used spannable/editable we should be able to update the spans then call invalidate() or requestLayout(), depending on the change, without needing to call setText again.

This looks promising but would probably be quite a large change. To verify that this is viable I will work on a little sample project as I get time. However I wanted to provide an update in case any of the authors or contributors have knowledge to share on this subject.

hypest commented 5 years ago

The multiple calls to setText seem to be due to the multiple images in the HTML being loaded and setText getting called each time one is ready for display.

👋 @drspaceboo , can you share here a "stacktrace" or call graph of such a case to have a look? Thanks!

cassgenerator commented 5 years ago

@hypest It should be visible in the trace file linked in the original comments.

I would grab more but I'm away from my computer until Monday. All I did to see it today was log a stack trace when refreshText is called.