463066 - removing comments from all evaluated source files (js, css, xul) for startup times improvments in fennec

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Frederic Plourde (:fred23)]

User-Agent:       Opera/9.52 (X11; Linux i686; U; fr-CA)
Build Identifier: 

removing comments from all evaluated source files (js, css, xul). 
If there's less to read in, we should gain on startup time.

Reproducible: Always

Actual Results:  
I intend to write a python script called something loke "removeComments.py" that removes all // and /* comments from .css, .js and .xul scripts.

This script should be called upon "package" make target and modify hard-copied scripts before wrap-up.

Expected Results:  
startup time reduction in Fennec.

Comment 1

[Frederic Plourde (:fred23)]

Already some results :

I have written a short script that effectively removes most of script comments. I have run preliminary startup times tests with stopwatch on my desktop and directly on the device, and here the mean startup times for 15 fennec startups : 

ON DESKTOP (X86) :  **************

BEFORE commentStripping:
	mean time : 9,9876
AFTER commentStripping
	mean Time : 9,5380
gain : 4.5 %


ON NOKIA N810 (ARMEL)**********
BEFORE commentStripping:
	mean time : 15.5127
AFTER commentStripping
	mean Time :  15.5037

gain : not significant

Comment 2

[Frederic Plourde (:fred23)]

Attachment: Python script that removes comments from a text file

Comment 3

[Oleg Romashin (:romaxa)]

I'm not sure that comments removing will bring a lot of improvements...
What do you think about function/variable renaming (making it shorter) ?

Comment 4

[Stuart Parmenter]

that would be bad for extensions

Comment 5

[Oleg Romashin (:romaxa)]

Do you mean that case when we rename public api functions/variables?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #5)
> Do you mean that case when we rename public api functions/variables?

Yes

Comment 7

[Frederic Plourde (:fred23)]

Preliminary results I conducted on Desktop show that we could gain from comments removal, since I got significant speedups (5% gain with very small standard deviation on the obtained startup times) and since most of the js and xul scripts have large proportions of comments (typically 30 - 50 %)..... But I guess the impact will go unnoticed at the present, since most of the time spent at N810's startup is taken elsewhere, possibly, as Stuart said, waiting for timeouts issued by OS2008...

My guess is that we should keep that "comments removal" script in mind for when startup time will be around 3 sec... then, the associated impact will certainly be noticeable.

Comment 8

[Antoine Azar [:antoine]]

Has this ever been incorporated? with the recent gains I'd be curious to see the improvement.

Comment 9

[Frederic Plourde (:fred23)]

I've retested that last week.
It's starting to show up on the stopwatch but still, it's a marginal gain. I'll use in-code timers soon to see if it's significant.

Comment 10

[Frederic Plourde (:fred23)]

I've revisited this "comments removal" bug.

  While it seemed interesting on desktop (see comment #1), recent tests using automated timers on the N810 showed no gain.

resulting averages for 10 readings :
-------------------------------------
with comments    ==> 8,758 +- 0.050 sec
without comments ==> 8,756 +- 0.040 sec

The obtained micro-gain of 2 ms is too tiny compared to the associated std deviation. So it's really UNsignificant.

IMO, this bug could either be closed... or I could still revisit it when startup time is closer to target 3 sec, see if we get different figures.

Comment 11

[BMO Automation]

Closing all opened bug in a graveyard component