Opened 16 years ago
Closed 15 years ago
#1118 closed defect (fixed)
inefficient comment stripping in buildUtil.js
Reported by: | Owned by: | alex | |
---|---|---|---|
Priority: | high | Milestone: | 0.9 |
Component: | BuildSystem | Version: | 0.2 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
buildUtil.js has the following function:
function removeComments(contents){ // if we get the contents of the file from Rhino, it might not be a JS // string, but rather a Java string, which will cause the replace() method // to bomb. contents = new String((!contents) ? "" : contents); // clobber all comments contents = contents.replace( /^(.*?)//(.*)$/mg , "$1"); contents = contents.replace( /( )/mg , "__DOJONEWLINE"); contents = contents.replace( //*(.*?)*//g , ""); return contents.replace( /__DOJONEWLINE/mg , " "); }
It appears that the purpose of the DOJONEWLINE replacements is to get around the (.*?) regex not matching newlines. A much simpler (and faster) method is to use ([sS]*?), that is, match all whitespace and non-whitespace characters (and thereby including newlines). This results in the much more compact version of the function:
function removeComments(contents){ // if we get the contents of the file from Rhino, it might not be a JS // string, but rather a Java string, which will cause the replace() method // to bomb. contents = contents ? new String(contents) : ""; // clobber all comments return contents.replace( /(/*([sS]*?)*/|//(.*)$)/mg , ""); }
Apart from getting rid of the DOJONEWLINE hack, this version is also significantly faster: it saved around 3 seconds off of our compile time. (String manipulation in Javascript is *very* slow.)
Change History (2)
comment:1 Changed 16 years ago by
Milestone: | → 0.5 |
---|
comment:2 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Note: See
TracTickets for help on using
tickets.
(In [9707]) Thanks, Tim. Shaved about a second off a base build for me. Amazingly, the year-old patch still applies. Fixes #1118