Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Only uglify javascript with a newline in it.

Summary:
I added comments at the top of the file explaining why we uglify at
all, and why uglifying is only necessary when the javascript has
newlines.

This gives a nice speedup to 'make pack:

   % make pack
   old: 92.293u 9.616s 1:52.15 90.8%    0+0k 16+7712io 0pf+0w
   new: 71.260u 8.792s 1:36.97 82.5%    0+0k 0+7896io 0pf+0w

23% less cpu time!  Most of this is spent spawning pack.rb, so in
streaming mode (which deploy.py uses), it's even better:

   % cat exercises/*.html | ruby build/pack.rb >/dev/null
   old: 28.197u 0.564s 0:31.92 90.0%    0+0k 0+0io 0pf+0w
   new: 7.604u 0.116s 0:08.49 90.8%     0+0k 0+0io 0pf+0w

A whopping 75% less cpu time!

Small style cleanup suggested in review.

Test Plan:
Did a diff of exercises-packed with both the old and new pack.rb.
Diffs were as expected:
   -        <var id="INT">rand(5)&gt;0?randRange(1,9):0</var>
   -        <var id="FRAC">rand(3)&gt;0?"."+randRange(1,9):""</var>
   -        <var id="SIGN">randFromArray(["","-"])</var>
   +        <var id="INT">(rand(5) &gt; 0 ? randRange( 1, 9 ) : 0)</var>
   +        <var id="FRAC">(rand(3) &gt; 0 ? "." + randRange(1, 9) : "")</var>
   +        <var id="SIGN">(randFromArray(["", "-"]))</var>
   <etc>

I also ran python -m SimpleHTTPServer and visited
   http://localhost:8000/exercises-packed/absolute_value_equations.html
   http://localhost:8000/exercises-packed/age_word_problems.html
   http://localhost:8000/exercises-packed/angles_2.html
and verified they still render correctly.

Reviewers: alpert, eater

Reviewed By: alpert

Differential Revision: http://phabricator.khanacademy.org/D2099
  • Loading branch information...
commit afd017a91f3731138926730b3525d07097addc69 1 parent 050dcce
@csilvers csilvers authored
Showing with 34 additions and 5 deletions.
  1. +34 −5 build/pack.rb
View
39 build/pack.rb
@@ -1,4 +1,25 @@
-# Packs exercise files. Usage modes:
+# Packs javascript in exercise files, and does some sanity-checking.
+#
+# The sanity-checking is to make sure that the javascript nodes are
+# well-formed: they don't have html children, etc.
+#
+# The packing is to make sure the javascript nodes are well behaved.
+# The problem is that we put javascript into nodes that expect to
+# have html: some <div>'s, the non-standard <var> tag, etc. Some
+# browsers, notably IE8, do some whitespace normalization on these
+# tags, thinking they're HTML. This is a problem for javascript,
+# where newlines can have meaning (they're equivalent to ; in some
+# contexts, and they terminate //-style comments). By uglifying
+# the javascript first, we normalize it to a form where whitespace
+# is *not* meaningful: uglifying strips out //-style comments, and it
+# inserts ; every place newlines are implicitly substituting for ;.
+#
+# The above suggests there's no need to uglify js with no newlines,
+# and indeed we avoid doing that, for efficiency.
+
+
+
+# Usage modes:
# 1) Pass a single filename on the commandline. Output is written
# to stdout.
# 2) Pass the contents of a single file via stdin. Output is
@@ -77,6 +98,14 @@ def jshint(js)
end
end
+def uglify(js)
+ if js =~ /\n/
+ return @uglifier.compile(js)
+ else
+ return js
+ end
+end
+
# All paths are relative to khan-exercises/ root
Dir.chdir(File.join(File.dirname(__FILE__), ".."))
@@ -117,7 +146,7 @@ def pack_file(file_contents)
jshint("return (#{var.content});")
exp = "(#{var.content})"
- var.content = @uglifier.compile(exp).gsub(/;$/, "")
+ var.content = uglify(exp).gsub(/;$/, "")
end
doc.css(".graphie", "div.guess", "div.show-guess", "div.show-guess-solutionarea").each do |graphie|
@@ -127,7 +156,7 @@ def pack_file(file_contents)
end
js = graphie.content
- graphie.content = @uglifier.compile(js).gsub(/;$/, "")
+ graphie.content = uglify(js).gsub(/;$/, "")
end
doc.css("div.validator-function").each do |validator|
@@ -139,7 +168,7 @@ def pack_file(file_contents)
# Need to wrap validator-function content in a function, so uglifier
# doesn't get confused by the estranged 'return' statement
js = "(function(){" + validator.content + "})()"
- uglified = @uglifier.compile(js)
+ uglified = uglify(js)
# Strip out the anonymous function wrapper to put things back the way they were
match = uglified.match(/^\(function\(\)\{(.*)\}\)\(\);?$/)
@@ -154,7 +183,7 @@ def pack_file(file_contents)
doc.css("[#{data_attr}]").each do |el|
jshint("return (#{el[data_attr]});")
js = el[data_attr]
- el[data_attr] = @uglifier.compile(js).gsub(/;$/, "")
+ el[data_attr] = uglify(js).gsub(/;$/, "")
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.