Skip to content
This repository was archived by the owner on Feb 20, 2023. It is now read-only.

0.2-0.3 MiB allocated per keystroke on awesomescreen #21293

Closed
mcomella opened this issue Sep 14, 2021 · 5 comments
Closed

0.2-0.3 MiB allocated per keystroke on awesomescreen #21293

mcomella opened this issue Sep 14, 2021 · 5 comments
Labels
needs:triage Issue needs triage performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Sep 14, 2021

Discovered in #21171 (comment)

Steps to reproduce

  • Attach AS profiler to memory view
  • Open search screen
  • Type a character

Expected behavior

Small increase in memory

Actual behavior

0.2 - 0.3 MiB allocated. That's 1 MiB per 5 characters. This seems excessive but I don't have the intuition to know for sure.

We should investigate what is being allocated and if it can be removed because these allocations use memory that may slow down later parts of the app, they cause GCs, and they may use more power (especially via GCs). A non-trivial amount of these allocations may come from #21294

Device information

  • Android device: Moto G5
  • Fenix version: main around 52975b4

┆Issue is synchronized with this Jira Task

@mcomella
Copy link
Contributor Author

mcomella commented Sep 17, 2021

I ran the AS allocation tracker with the following steps:

  • initial state: search screen opened with "a" typed
  • type "s"

The major improvements I see are:

  • (separate issue?) AutoSave uses ~0.22 MiB each time it's called. Excluding that from the results...
  • 20.3% of allocated memory: Layout traversals, which seem unnecessary on this screen. Would help CPU/power to remove them too
  • 19.5%: Response.Body.string(). The network requests allocate two buffers of default size 16 KiB but could use the content-length header to allocate smaller buffers
  • 7.9%: BrowserAwesomeBar$queryProvidersForSuggestions. 43% of this method is Fact.toEvent is called many times & generally allocates 35 Pairs each time: power use issue #21294 which is addressing a when statement invocation

Here's the full summary of what I found in the allocation tracker:

  • 495256 bytes allocated total
  • 232080 bytes via AutoSave$triggerSave. Is this time based or do we trigger ?
  • excluding autoSave which might be time based (though I see it often when doing allocation tracking here), 263176 bytes remain:

Threads:

  • Main thread: 103616 bytes
    • 53304 bytes = 36176 + 8464 + 4928 + 3736 bytes from doTraversal-ish
    • 20800 bytes from BrowserAwesomeBar$queryProvidersForSuggestions
      • 8928 bytes from emitAwesomebarFact
      • 8392 bytes from addSuggestions
    • 10104 bytes for setComposingText
  • BrowserAwesomeBar-thread-1: 64664 bytes
    • 51272 from Response.Body.string()
      • 22240 Reader.readText
      • 20480 BufferedReader.
      • 8480 InputStreamReader.

@mcomella
Copy link
Contributor Author

mcomella commented Sep 17, 2021

19.5%: Response.Body.string(). The network requests allocate two buffers of default size 16 KiB but could use the content-length header to allocate smaller buffers

I attempted an implementation of my suggested fix in this branch but writing the tests took a while (and they feel necessary for such critical code) so I stopped working on it.

@mcomella
Copy link
Contributor Author

I used the app and monitored the memory allocations: I never noticed the GC causing disruptions in the UI and in general, the JVM allocations are tiny compared to the allocations from the native code so perhaps JVM memory isn't a major issue, at least compared to runtime delays users perceive (e.g. clicking a button and a long pause before it happens).

@mcomella
Copy link
Contributor Author

The major improvements I see are:
...
* 19.5%: Response.Body.string(). The network requests allocate two buffers of default size 16 KiB but could use the content-length header to allocate smaller buffers

I filed mozilla-mobile/android-components#11015 to make this bite-sized.

* 20.3% of allocated memory: Layout traversals, which seem unnecessary on this screen. Would help CPU/power to remove them too

This code is changing so we'll have to remeasure later. We're actively looking at the search screen so we don't need to file a new issue.

* 7.9%: BrowserAwesomeBar$queryProvidersForSuggestions. 43% of this method is [Fact.toEvent is called many times & generally allocates 35 Pairs each time: power use issue #21294](https://github.com/mozilla-mobile/fenix/issues/21294) which is addressing a `when` statement invocation

An issue & a PR is opened for this one.

* (separate issue?) AutoSave uses ~0.22 MiB each time it's called. Excluding that from the results...

Let's revisit this one next triage. We can file a new issue if folks think its an issue. After that, we can close this issue.

@mcomella
Copy link
Contributor Author

mcomella commented Oct 6, 2021

I want to clean up this issue so it's easier to triage so I'll go through the breakdown again.

(separate issue?) AutoSave uses ~0.22 MiB each time it's called. Excluding that from the results...

Let's file this when we're more confident memory is an issue.

20.3% of allocated memory: Layout traversals, which seem unnecessary on this screen. Would help CPU/power to remove them too

This is changing so we'll refile if we see issues later.

19.5%: Response.Body.string(). The network requests allocate two buffers of default size 16 KiB but could use the content-length header to allocate smaller buffers

Improved in mozilla-mobile/android-components#11015 and open follow-up in mozilla-mobile/android-components#11100

7.9%: BrowserAwesomeBar$queryProvidersForSuggestions. 43% of this method is
Fact.toEvent is called many times & generally allocates 35 Pairs each time: power use issue #21294 which is addressing a when statement invocation

Partially addressed in #21294

So I think we can close this.

@mcomella mcomella closed this as completed Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs:triage Issue needs triage performance Possible performance wins
Projects
None yet
Development

No branches or pull requests

1 participant