Right now, due in part to the JS engine not parsing strings very well, we're ending up with a huge number of copies of this logo. Plus data URIs are not a very efficient way to store things. It would be nice if it could be stored more efficiently, say in a binary format.

How do we read a binary file on the filesystem from a content page?

Assuming we fix the JS engine to parse strings better, how much of a memory-use win would fixing this be? I'm skeptical that it's worth the trouble.

base64 encoding is an inefficient way to store images.

The full string is 9258 characters long, and each character takes a couple of bytes to represent in the JS engine, IIRC, so that's around 18kb, which isn't too terrible. We also should figure out why there are two copies of the full string, as it would be bad if the number of copies of it is unbounded. Anyways, I agree that it may not be worth the hassle of encoding it in a binary format, which is why I left this fairly open-ended. The "right" thing to do in the short term might even just be to make this giant string into a single line in aboutHome.js.

Inefficiency isn't enough to outweigh convenience - needs to be inefficient and significant :)

I'm seeing where we end up if I convert this into a single multiline string.

I used njn's suggestion in bug 986628 comment 13 and turned this into a single multiline string. If my logging can be believed, this gets rid of every copy of the string from memory (?!?).

why not also on the "data:image/png;base64," line?

Comment on attachment 8395062 [details] [diff] [review] Use a single multiline string for the search engine logo to avoid startup memory bloat this is a no-brainer if it helps (probably even if it doesn't!) (as mak says, first line can use the same treatment)

Oops, sorry, there are zero copies of this because I bungled this somehow and it produces an error and doesn't show the image. :( JavaScript error: chrome://browser/content/abouthome/aboutHome.js, line 10: illegal character

In Gaia there was a period where seemingly every binary asset (pictures, ringtones, etc.) was stored as a base64 data URI, and it was terrible. So forgive me if I'm leery about the practice! Hopefully it's not being used for larger assets elsewhere.

Comment on attachment 8395062 [details] [diff] [review] Use a single multiline string for the search engine logo to avoid startup memory bloat *ahem*

what if we make an about:defaultsearchenginelogo redirect pointing to a binary image? with URI_SAFE_FOR_UNTRUSTED_CONTENT may work

That'd have to be web-exposed, which isn't ideal (though maybe not a big deal in practice).

Fixed version. I guess that explains why the highlighting in Emacs was all weird!

So, here's an alternate approach. It turns out we can load chrome URI images from content (!?!?), so we could actually just load this as a file. Here's a proof of concept that uses a chrome URI image I found somewhere, and I can confirm that it works.

Should depend on whether the package is contentaccessible, but yeah that should work.

I think at a certain point we also had an about:home mockup that was just using the search engine logo (the same we use in the search bar) just with higher DPIs... so we could also go further, pass the icon from the search service along with the engine, so we have icons for any engine. Worth involving UX and see if we can stop having a large logo just for google? (actually, that may also require marketing, I think there was an agreement with google about the use of the current logo).

After some minor hijinx where I set the image source to the string "Google", I tested this and it properly displays the Google logo. In addition, we end up with exactly one copy of this string in memory, in a GC log taken before the first startup GC, as expected. I added a little note explaining the oddness, so nobody decides it is ugly and undoes it.

I instrumented the ConcatString() call in frontend/FoldConstants.cpp, printing out the size of the resulting string. If I sum all the lengths before applying this patch, I get 1,727,384 bytes (and that's just doing length()*sizeof(jschar) -- I haven't accounted for any slop bytes). After applying the patch, it drops to 266,702 bytes. Good! The output shows me that there are still some other strings like this one, albeit smaller. E.g. there's one that spans 40 lines. I'll try to hunt them down.

> The output shows me that there are still some other strings like this one, > albeit smaller. E.g. there's one that spans 40 lines. I'll try to hunt them > down. I added some instrumentation to give me JS stacks where these are occurring, and a lot of these seem to be coming from within Cu.import(), CC[].getService() and Cu.evalInSandbox() calls. gavin, how are they implemented?

I'm not sure what you're asking, or what you mean exactly by "from within Cu.import() calls".

I added some hacky instrumentation to SpiderMonkey to get the JS stack each time the concatenation occurs. In most cases it pointed to a line containing a Cu.import() call. For example, line 226 of XPCOMUtils.jsm was mentioned frequently, and it looks like this: Cu.import(aResource, temp); I'm not sure how to interpret this -- maybe the instrumentation is bogus, or maybe the implementation of Cu.import() is triggering the concatenations somehow. So I'd like to look "inside" Cu.import(), if that makes sense.

It sounds like maybe the instrumentation is telling you that a script that is being import()ed is what contains the long string? Cu.import is implemented in mostly mozJSComponentLoader::ImportInto (via mozJSComponentLoader::Import).

> Cu.import is implemented in mostly mozJSComponentLoader::ImportInto (via > mozJSComponentLoader::Import). Mmm, that appears to be what's happening. Thanks.

This is probably one of the causes of the increase in pre-settled startup memory in the last year or so. Bug 818940, which landed in 23, increased the size of this image from about 60 lines to about 120 lines. I could calculate how much of a memory increase it is using a sum or something but I am lazy. try run: https://tbpl.mozilla.org/?tree=Try&rev=16d6e497ede6

This is kind of initially a regression from bug 627301, which landed in Firefox 4. Before that, the data URI was represented as a single string.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f3d8044e709 I guess this should technically land on the Fx inbound or whatever but this file seems to rarely change so hopefully it will be okay...