702642 - DOMTemplate is relatively slow when evaluating JS ${}

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

DOM Templater is relatively slow when evaluating JS ${}. Joe says he has a fix for this.

Comment 1

[Joe Walker [:jwalker]]

I'll also pre-compile the regexes while I'm at it.

Comment 2

[Dave Camp (:dcamp)]

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]]

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]]

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)]

Those changes look good to me.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Look fine to me too

Comment 7

[Joe Walker [:jwalker]]

Attachment: upload 1

Should be automatic given the comments expressed so far

Comment 8

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=28134b5df88a

Comment 9

[Joe Walker [:jwalker]]

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)]

Is this ready to land?

Comment 11

[Joe Walker [:jwalker]]

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)]

OK. Signal when ready.

Comment 13

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=cf3ed4316481

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/f1304b596193