Ben Kelly [:bkelly, not reviewing] Reporter
	Description • 11 years ago

The ril_worker.js uses about 3x the memory net_worker.js uses on my device:

├───3.82 MB (10.24%) -- workers/workers()
│   ├──2.85 MB (07.66%) ++ worker(resource://gre/modules/ril_worker.js, 0x443a6c00)
│   └──0.96 MB (02.58%) ++ worker(resource://gre/modules/net_worker.js, 0x443a7400)

Its possible this difference is due to a larger workload for ril_worker.js, but perhaps there are optimizations we could make here.

For example, can we gain some memory back by not loading require.js and importing other modules directly, etc.

	[:fabrice] Fabrice Desré
	Comment 1 • 11 years ago

Note that the ril worker is a massive amount of code. Almost 1MB is due to script-sources, and that should go down once bug 944659 lands.

	[:fabrice] Fabrice Desré
	Comment 2 • 11 years ago

This patch implements Ben's idea of not using require.js to load worker_buf.js
The memory saving is modest (about 60k per worker).

	Nicholas Nethercote [inactive]
	Comment 3 • 11 years ago

Do we have any ideas for optimizations that are specific to ril_worker.js? If not, I think we should just dup this bug against bug 864927.

	[:fabrice] Fabrice Desré
	Comment 4 • 11 years ago

There's the simple patch I attached there that is ril-worker specific. Not a huge gain, but still a little something.

	Nicholas Nethercote [inactive]
	Comment 5 • 11 years ago

If that's it, the title should be updated accordingly. Thanks!

	Vicamo Yang [:vicamo][:vyang]
	Comment 6 • 11 years ago

(In reply to Fabrice Desré [:fabrice] from comment #4)
> There's the simple patch I attached there that is ril-worker specific. Not a
> huge gain, but still a little something.

We can integrate worker_buf.js back to ril_worker again once bug 933588, rewrite nfc_worker with C++, is merged.

	[:fabrice] Fabrice Desré
	Comment 7 • 11 years ago

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #6)
> (In reply to Fabrice Desré [:fabrice] from comment #4)
> > There's the simple patch I attached there that is ril-worker specific. Not a
> > huge gain, but still a little something.
> 
> We can integrate worker_buf.js back to ril_worker again once bug 933588,
> rewrite nfc_worker with C++, is merged.

Absolutely Vicamo, but I'd rather not wait for that. I'll update the patch to also fix the nfc worker.

	Ben Kelly [:bkelly, not reviewing] Reporter
	Comment 8 • 11 years ago

Well I had other ideas to investigate, but I guess I'll just write new bugs if they pan out.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 9 • 11 years ago

mark 1.3T?

	Joe Cheng [:jcheng] (please needinfo)
	Comment 10 • 11 years ago

triage: saving is small. not block tarako on this

	Nicholas Nethercote [inactive]
	Comment 11 • 11 years ago

Fabrice, should you request review on the patch?

	[:fabrice] Fabrice Desré
	Comment 12 • 11 years ago

(In reply to Nicholas Nethercote [:njn] from comment #11)
> Fabrice, should you request review on the patch?

I need to rebase after the big RIL worker refactoring from bug 960894 first.

	Eric Rahm [:erahm]
	Comment 13 • 7 years ago

Closing out old Firefox OS specific memshrink bugs.