1216903 - Use rollup.js for building

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::L10n, defect)

Description

[Staś Małolepszy :stas]

Our custom babel bundler does a good job of bundling modules together into a single file but it does so at the expense of creating 16 or 17 "module" functions.  Memory-wise, it would be cheaper to concatenate the modules into a flat file and making sure there are no naming conflicts.

I came across rollup.js which seems to do exactly that.  It also has a cool feature called 'tree-shaking' which analyzes the whole code and excludes code that is not needed.

http://rollupjs.org
https://www.npmjs.com/package/grunt-rollup

Here's an example of the output:

http://rollupjs.org/#{%22options%22:{%22format%22:%22iife%22,%22moduleName%22:%22myBundle%22,%22globals%22:{}},%22modules%22:[{%22name%22:%22main.js%22,%22code%22:%22import%20{%20cube%20}%20from%20%27./maths.js%27;\nconsole.log%28%20cube%28%205%20%29%20%29;%20//%20125%22},{%22name%22:%22maths.js%22,%22code%22:%22//%20This%20function%20isn%27t%20used%20anywhere,%20so\n//%20Rollup%20excludes%20it%20from%20the%20bundle...\nexport%20function%20square%20%28%20x%20%29%20{\n\treturn%20x%20*%20x;\n}\n\n//%20This%20function%20gets%20included\nexport%20function%20cube%20%28%20x%20%29%20{\n\t//%20rewrite%20this%20as%20%60square%28%20x%20%29%20*%20x%60\n\t//%20and%20see%20what%20happens!\n\treturn%20x%20*%20x%20*%20x;\n}%22}]}

Comment 1

[Staś Małolepszy :stas]

Attachment: Pull request

This was pretty easy to hook up into our build system, now that we only had our custom bundler.

I did a raptor run for FM Radio with n=30 and got good results with very low stdevs for perf and memory (50KB on master and 25KB with the patch).

fm.gaiamobile.org      base: mean  1: mean  1: delta  1: p-value
---------------------  ----------  -------  --------  ----------
navigationLoaded              527      511       -16        0.14
navigationInteractive         736      727        -8        0.37
visuallyLoaded                736      728        -9        0.36
contentInteractive            736      728        -8        0.37
fullyLoaded                   911      910        -1        0.93
uss                        10.805   11.060    -0.255      * 0.00
pss                        14.874   15.124    -0.250      * 0.00
rss                        29.441   29.712    -0.271      * 0.00

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Love it!

Can you:
 - rename 'fetch' function that is currently conflicting with Fetch API
 - modify rollup to remove comments
 - look for a way to prevent rollup from changing unicode characters into \uXXXX representation?

It seems that the current patch increases l20n.js result file by over 3kb, while it should significantly reduce the size because all the boilerplate code and import/export lines are gone. It seems that majority of the increase is in comments/unicode chars.

Comment 3

[Staś Małolepszy :stas]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)

> Can you:
>  - rename 'fetch' function that is currently conflicting with Fetch API

Sure, is fetchResource ok?

>  - modify rollup to remove comments

I actually think it's a feature of rollup that the comments are preserved.  We can then minify the code and strip the comments as needed, but I like it that the rollup output contains them.  I even thought of adding a banner comment which tells Gaia devs not to edit the shared/js/intl/l20n.js file and file bugs against upstream repo instead.

>  - look for a way to prevent rollup from changing unicode characters into
> \uXXXX representation?

Could you give an example of this?  I'm not sure I understand what you mean.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas from comment #3)
> (In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> 
> > Can you:
> >  - rename 'fetch' function that is currently conflicting with Fetch API
> 
> Sure, is fetchResource ok?


Yeah, works for me.


> >  - modify rollup to remove comments
> 
> I actually think it's a feature of rollup that the comments are preserved. 
> We can then minify the code and strip the comments as needed, but I like it
> that the rollup output contains them.  I even thought of adding a banner
> comment which tells Gaia devs not to edit the shared/js/intl/l20n.js file
> and file bugs against upstream repo instead.

Hmm, ok. I can see that. Good point.

> >  - look for a way to prevent rollup from changing unicode characters into
> > \uXXXX representation?
> 
> Could you give an example of this?  I'm not sure I understand what you mean.

Seems like rollup is modifying unicode chars. Example is `charMaps` switched from:

'ȦƁƇḒḖƑƓĦĪ' to '\u0226\u0181\u0187\u1E12\u1E16\u0191\u0193\u0126\u012A'

Comment 5

[Staś Małolepszy :stas]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)

> Seems like rollup is modifying unicode chars. Example is `charMaps` switched
> from:
> 
> 'ȦƁƇḒḖƑƓĦĪ' to '\u0226\u0181\u0187\u1E12\u1E16\u0191\u0193\u0126\u012A'

it's how it's written in the source file:

https://github.com/l20n/l20n.js/blob/5bae3ebb13ad87c1c7cece1f441880a202135ea3/src/lib/pseudo.js#L97

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

hmm. so it was babel that was minifying it. Ok. We may want to switch it in the source, but please, don't block landing this on that.

Comment 7

[Staś Małolepszy :stas]

Ah, interesting.  I filed bug 1217312 to fix that.

I'll rename fetch to fetchResource and will land this today.

Comment 8

[Staś Małolepszy :stas]

https://github.com/l20n/l20n.js/commit/5504d409b7229d15aab637283a938f7f631394a9

Comment 9

[Staś Małolepszy :stas]

I tested FM Radio on flame-kk again, this time with n=60.  Looks like we're about to see a nice memory win. 

fm.gaiamobile.org      base: mean  1: mean  1: delta  1: p-value
---------------------  ----------  -------  --------  ----------
navigationLoaded              583      569       -14      * 0.03
navigationInteractive         817      804       -12        0.29
visuallyLoaded                817      805       -12        0.29
contentInteractive            818      805       -12        0.30
fullyLoaded                  1014      999       -15        0.33
uss                        11.082   10.830    -0.252      * 0.00
pss                        15.115   14.857    -0.257      * 0.00
rss                        29.685   29.433    -0.252      * 0.00