768697 - Loading the Mail app is slow

Status: RESOLVED DUPLICATE of bug 892069

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

It takes a long time the Mail app for the first time.

Here's a profile of it happening:

http://people.mozilla.com/~bgirard/cleopatra/?report=AMIfv95FHknlYUxY367dK8z4cLWD_mnZbxYuR-GeUIRpxA4a-ATc7EahoRALPUsdVdC8YQZU6EVgDv0xLgR1evrAxD0r5TG-WYRSLO1R4ezPhOulySGQr9X9ZnKM4lHX1y-2U0I9BR6wh6ktcrUFu7h5thO4yXY6yQ

Notable things:
 - 1.6 seconds in JS::EvaluateString
 - 1.4 seconds in JS:CallEventHandler
   - 69% of the time is spent in js::types::TypeCompartment::markSetsUnknown()
 - main thread sqlite calls

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Thanks for the profile!

- Our main e-mail JS lib is 1.3 megabytes right now, which probably explains the JS::EvaluateString.  (I am assuming <script> loads go via JS::EvaluateString...).

The JS lib is just a concatenation of other JS files; comments are not stripped, and no minification is performed.  We can probably shrink the file size a good deal when we do that.


- It looks like the markSetsUnknown may be associated with mutating __proto__.  The only instance in the code I found of this is from node's util.js's inspect helper, which is just in there for shim dependency reasons.  I'll remove that and hopefully that makes things faster.  I do have uses of __proto__ elsewhere but only during initial object creation (no mutation!) and only referencing objects that won't be further mutated.  Ex:

  function Bob() {}
  Bob.prototype  = {...};
  function Foo() {}
  Foo.prototype = {
    __proto__: Bob.prototype,
  };

I can switch to Object.create for these cases if required...


- We don't use localStorage, so I'm not sure what is causing the main-thread SQLite.  There don't seem to be any stack frames providing blame that I can see...?

Comment 2

[Andreas Gal :gal]

Awesome profile. Thanks Jeff! andrew, automatic r- from me on any code containing "__proto__". Use Object.create. __proto__ is very expensive in the JS engine, and not needed these days.

Jeff, I am betting the sqlite stuff is offline cache reading (see bug 767025). Bonus points if you can confirm that.

Why is the JS lib so huge? The event handler calls seem nuts too.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Andreas Gal :gal from comment #2)
> Jeff, I am betting the sqlite stuff is offline cache reading (see bug
> 767025). Bonus points if you can confirm that.

The hack described in bug 767025 comment 6 should help confirm.

Comment 4

[Mike Hommey [:glandium]]

(In reply to Andrew Sutherland (:asuth) from comment #1)
> Thanks for the profile!
> 
> - Our main e-mail JS lib is 1.3 megabytes right now, which probably explains
> the JS::EvaluateString.  (I am assuming <script> loads go via
> JS::EvaluateString...).

Another thing to take into account is that from my observation, the flash is slow, even for reading, so it might actually be a great win if we'd compress the offline cache data. (There's a 22MB file that looks like a japanese dictionary for IME that I'm pretty sure takes around 10 seconds to load)

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Andreas Gal :gal from comment #2)
> Awesome profile. Thanks Jeff! andrew, automatic r- from me on any code
> containing "__proto__". Use Object.create. __proto__ is very expensive in
> the JS engine, and not needed these days.

All uses of __proto__ have now been removed; this is landed on gaia.

> Why is the JS lib so huge? 

608k of this is the string encoding polyfill from http://code.google.com/p/stringencoding/.  If someone lands a native version of http://wiki.whatwg.org/wiki/StringEncoding, that goes away (and we probably get a lot faster, although JIT improvements will probably bear extensive fruit as-is).

The next largest libs are IMAP sync logic (96k) and IMAP protocol logic (73k).  The sync logic contains a lot of comments that will eventually be stripped out; uglify.js shrinks IMAP sync logic down to 25k, for example.

There are one or two files I should be able to easily remove that are needed for unit tests that are getting pulled in because of stated but unused dependencies.  With a little more effort I can create an alternate version for production that cuts out the dead code that results in those stated dependencies.

The lib as-is currently gzips down to 378k

> The event handler calls seem nuts too.

That includes the markSetsUnknown time.

Much of app startup also has to happen in an event handler because our configuration comes out of IndexedDB and we need to wait on the 'localized' event notification.

Comment 6

[Andreas Gal :gal]

Is the string code needed during app opening/startup or can you load that lazily?

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Andreas Gal :gal from comment #6)
> Is the string code needed during app opening/startup or can you load that
> lazily?

It can be lazily loaded.  Charset conversion only happens when processing or creating e-mail messages or speaking to the IMAP server.

Would you prefer a total lazy load where a shim triggers the load the first time a TextEncoder/TextDecoder is created, or an opportunistic one that is initiated once the UI has completed startup?

The primary difference is that when the device is offline, the totally lazy solution would not need to load the library at all until a message is composed or the network comes back online.  In the online case, UI startup of the (already configured) mail app is gong to try and hit the network as part of the process to refresh the Inbox.

Comment 8

[Andreas Gal :gal]

I would probably do it when you instantiate TextEncoder/TextDecoder. Other parts of the JS code too until startup is pretty much instant. We can chat about this offline.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We're working on implementing the encoding/decoding API in bug 764234. But there's no ETA right now.

Comment 10

[Andrew Sutherland [:asuth] (he/him)]

Bug 800396 removes the TextEncoder/TextDecoder poly-fill.  It is waiting on approval to land.  This will take our concatenated JS file from 1.7M to 1.1M.

That was far and away the lowest hanging fruit for startup latency.  Other than stripping out comments or minifying the JS to reduce I/O burden and parse burden, most other wins will require profiling and analysis.  We have an intentional UX goal right now to not jerk around the user by showing them old messages that will immediately scroll off the screen once we talk to the server, so pushing out our network traffic by lazy loading protocols and such may not be a net win.  NB: There are explicit heuristics of when we have synced "recently enough" so that we can do a pure database load and then issue a refresh.