Last Comment Bug 702642 - DOMTemplate is relatively slow when evaluating JS ${}
: DOMTemplate is relatively slow when evaluating JS ${}
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: Firefox 11
Assigned To: Joe Walker [:jwalker] (needinfo me or ping on irc)
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-15 08:27 PST by Michael Ratcliffe [:miker] [:mratcliffe]
Modified: 2011-12-08 21:25 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
upload 1 (4.95 KB, patch)
2011-11-18 09:07 PST, Joe Walker [:jwalker] (needinfo me or ping on irc)
dcamp: review+
Details | Diff | Splinter Review

Description Michael Ratcliffe [:miker] [:mratcliffe] 2011-11-15 08:27:34 PST
DOM Templater is relatively slow when evaluating JS ${}. Joe says he has a fix for this.
Comment 1 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-15 09:46:09 PST
I'll also pre-compile the regexes while I'm at it.
Comment 2 Dave Camp (:dcamp) 2011-11-15 15:24:55 PST
Would it make sense to have different syntax to explicitly opt-in to an eval rather than a property lookup?
Comment 3 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-16 06:47:02 PST
Here's what I'm working on:
Old:

  new Templater().processNode(node, data);

New:

  template(node, data, options);

Where the latter just does 'new Templater(options).processNode(node, data);' behind the scenes.
There is a point to having an object (it allows you to cancel a asynchronous operations, etc). Previously you had to do:

  var t = new Templater();
  t.processNode(node, data);

Now:

  var t = template(node, data);

If 'new Template()' is called without any options, then it sets allowEval:true for backwards compat, however the default for the template() method is allowEval:false
Comment 4 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-16 07:57:05 PST
Mike / Dave - would be grateful for some feedback/review of the changes to DOM Template that we recently discussed: For the commits, see https://github.com/joewalker/domtemplate/commits/master for 'Nov 16, 2011'.
Comment 5 Dave Camp (:dcamp) 2011-11-16 13:55:25 PST
Those changes look good to me.
Comment 6 Michael Ratcliffe [:miker] [:mratcliffe] 2011-11-17 03:44:22 PST
Look fine to me too
Comment 7 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-18 09:07:33 PST
Created attachment 575482 [details] [diff] [review]
upload 1

Should be automatic given the comments expressed so far
Comment 8 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-21 08:33:20 PST
https://tbpl.mozilla.org/?tree=Try&rev=28134b5df88a
Comment 9 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-22 02:42:20 PST
Failures in previous try were not down to this patch, but just in case:
https://tbpl.mozilla.org/?tree=Try&rev=587bd6d86427
Comment 10 Dave Camp (:dcamp) 2011-11-29 13:30:05 PST
Is this ready to land?
Comment 11 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-11-30 15:53:21 PST
Green on try: https://tbpl.mozilla.org/?tree=Try&rev=058542478c55

This could land if it's important for it to do so, but landing early could cause some unnecessary patch churn, which is slightly annoying for me in a separate repo.
Comment 12 Rob Campbell [:rc] (:robcee) 2011-11-30 17:24:09 PST
OK. Signal when ready.
Comment 13 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-12-08 06:06:18 PST
https://tbpl.mozilla.org/?tree=Fx-Team&rev=cf3ed4316481
Comment 14 Tim Taubert [:ttaubert] 2011-12-08 21:25:06 PST
https://hg.mozilla.org/mozilla-central/rev/f1304b596193

Note You need to log in before you can comment on or make changes to this bug.