Opened 10 years ago

Closed 10 years ago

#10152 closed task (wontfix)

_Widget: _applyAttributes() and getSettertAttributes() optimization

Reported by: bill Owned by: bill
Priority: high Milestone: 1.5
Component: Dijit Version: 1.4.0b
Keywords: Cc: Les
Blocked By: Blocking:

Description

_applyAttributes() calls this.attr(name, value) on every widget attribute that occurs in either attributeMap or has a custom setter (or both, which happens due to subclassing).

getSetterAttributes() computes the list of attributes with custom setters.

Two possible optimizations are:

1) modify getSetterAttributes() to make the entire list of attributes that occur either in (a) attributeMap or (b) have a custom setter.

2) optimizations on how to find which attributes have custom setters. There are a lot of ways to do that which I was playing with last night, many of which don't use isFunction() or regexp's at all.

I'm attaching a patch that needs more benchmarking and testing.

Attachments (1)

applyAttributes.patch (3.0 KB) - added by bill 10 years ago.

Download all attachments as: .zip

Change History (3)

Changed 10 years ago by bill

Attachment: applyAttributes.patch added

comment:1 Changed 10 years ago by bill

Cc: Les added
Owner: set to bill

comment:2 Changed 10 years ago by bill

Resolution: wontfix
Status: newclosed

I worked on this for a few days and I think it's better to drop it, at least until it occurs as part of a bigger refactor for 2.0.

The optimization was supposed to speed up widget instantiation, so it was to be useful when creating a lot of the same widget (like the 100 buttons that benchTool.html creates). But I'm not seeing any measurable effect.

More than that though, this code is very fragile. I tried to simplify things by calling attr() for every constructor parameter plus for attributes with non-blank default values:

var paramSet = dojo.delegate(getDefaultParams(this), this.params||{});
this.attr(paramSet);

where getDefaultParams() returned a subset of the prototype, for example for dijit.form.Button:

{
   type: "button"
}

The (cached) result of getDefaultParams() didn't have attributes like title, style, class, etc. since they don't need attr() calls on widget instantiation unless they have a value specified in the constructor params.

getDefaultParams() was efficient because it's only computed once per widget class, no matter how many instances of the widget were created.

This didn't work for a number of reasons:

  • attributeMap contains id, even though the prototype has a blank id. id needs to be pulled from "this", not from the prototype
  • since the parser demands that all attributes have explicit types, "blank" dates are represented in the _DateTimeTextBox prototype as new Date(""), but then in [12165] and [14404] there's some hackish code to convert the prototype's new Date("") into null in the instance.
  • DOMNodes differentiate between unset attributes and blank value attributes. For example <input> is different than <input name="">. The former doesn't submit the value with the form at all, but the latter causes submission w/a "" name, ex: foo.php?=bar

Widgets don't (currently) have a concept of attributes not being set. For example name and tabIndex are always set to some defined value, either "" or "foo". _applyAttributes() has specific code so that this.name=="" won't get applied to the DOM. myWidget.attr(name, "") presumably will cause that unusual behavior of submitting the value w/no name.

  • Efficiency issues: the above issue could be solved by attrToDom() calling domNode.removeAttribute() when this.name=="" or possibly when this.name===undefined (although that runs into the parser problem where it can't tell the type of name). But calling removeAttribute() unnecessarily on widget instantiation for dir, lang, class, style, title, name, tabIndex, etc. could be slow.
Note: See TracTickets for help on using tickets.