Ticket #2530248 (assigned enhancement)

Reporter


Ryan Grove
Opened: 05/10/11
Last modified: 08/22/13
Status: assigned
Type: enhancement

Owner


Jenny Donnelly
Target Release: BACKLOG
Priority: P4 (low)
Summary: Coalesce changes via setAttrs() into a single change event
Description:

The app framework is going to need a way to coalesce changes to multiple attribute values (via setAttr()) into a single change event.

An app model might have a large number of attributes and many of those may change at once. The default app view behavior (though this is really up to the implementer) is to re-render an entire view
when a change occurs, and it's no good to have to re-render a view for each individual attribute rather than only once for the batch.

Type: enhancement Observed in Version: development master
Component: Attribute Severity: S3 (normal)
Assigned To: Jenny Donnelly Target Release: BACKLOG
Location: Library Code Priority: P4 (low)
Tags: Relates To:
Browsers: N/A
URL:
Test Information:

Change History

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 07/27/11
  • status changed from new to accepted

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 07/27/11
  • milestone changed from 3.4.0 to 3.5.0

These are backlog bugs which didn't make it into any of the 3.4.0 sprints. Marking as 3.5.0 backlog for evaluation going into 3.5.0 sprint 1

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 11/7/11
  • estimated changed from 0 to 1
  • remaining changed from 0 to 1
  • sprint changed to sprint 1

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 11/8/11
  • priority changed from P3 (normal) to P4 (low)

P4 as far as Sprint 1 is concerned. It is a must have for 3.5.0

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/5/11
  • sprint changed from sprint 1 to sprint 3

Moving P4s out of sprint (was supposed to have done this after the planning meeting but forgot)

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 01/31/12
  • milestone changed from 3.5.0 to 3.6.0

Moving these out based on priority. Not enough time in Sprint 3

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 01/31/12

Moving these out based on priority. Not enough time in Sprint 3

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 04/4/12
  • sprint changed from sprint 3 to backlog

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 04/4/12
  • sprint changed from backlog to sprint 1

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 04/4/12
  • estimated changed from 1 to 1.5
  • remaining changed from 1 to 1.5

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/8/12
  • sprint changed from sprint 1 to sprint 2

Need to move to Sprint 2, since Sprint 1 ended up going mostly towards 3.5.1

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/16/12
  • completed changed from 0 to 1
  • remaining changed from 1.5 to 0.5

Pulled over the impl from Model for the most part.

I think Ryan and I discussed the transaction approach when it was implemented at the Model level, so nothing to really change there in terms of the implementation.

Need to address 2 concerns in moving it down to Attribute though. a) is solvable. I'm not sure b) is solvable without a performance hit.

a) At the attribute level, it seems like the name of the event needs to be something more specific than "change", since that's likely to conflict with impls. mixing in Attribute.

Thinking "attrChange", with the understanding that it rules out having an attribute called attr. Essentially I wanted something which conveys *Change.

I have it configurable currently on the prototype, so the component can change in back to "change" if required.

b) Performance hit if set() is also routed through setAttrs() which model currently does, so that it results in an "attrChange" event.

I think this is the right thing to do - since the developer listening for after("attrChange") expects to be notified even if a single attribute changes through the set() method.

However I'm concerned about doubling every attribute event fired (fooChange and attrChange). I'm not publishing the event to help with this (it's currently published with preventable:false, in Model, but if there's no defaultFn there's really no point publishing it with just preventable:false AFAIK), but even then, it seems heavy, until we make event firing closer to no-ops if no-one is listening.

Continuing to look at potential performance implications.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 06/6/12
  • completed changed from 1 to 1.5

There's a 40% increase on a Base based impl, setting 20 attributes, when set is routed through setAttrs (https://gist.github.com/2879339 added to src/base/tests/benchmark/base-benchmark.js) on Chrome. 20% on an iPad1 with iOS 5.

A. WITH set routed through setAttrs (2 events fired, no listeners):

Chrome:

1. 313 ops/sec
2. 292 ops/sec
3. 279 ops/sec

iOS 5, iPad1:

1. 48 ops/sec
2. 48 ops/sec
3. 49 ops/sec

B. WITHOUT set routed setAttrs (1 event fired, no listeners):

Chrome:

1. 398 ops/sec
2. 432 ops/sec
3. 421 ops/sec

iOS 5, iPad1:

1. 58 ops/sec
2. 59 ops/sec
3. 59 ops/sec

Based on the above, holding off rolling the setAttrs down to the Attribute layer, until I've had a chance to see if there's some low hanging performance fruit in Event, which we can get (especially for the case of no listeners) to not only offset the performance hit, but get a bit of a savings also.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 06/6/12
  • remaining changed from 0.5 to 0

Stashed work on my fork/branch for now (https://github.com/sdesai/yui3/tree/setAttrs). Will switch gears to work on Event.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 07/16/12
  • milestone changed from 3.6.0 to 3.NEXT

Moving to 3.NEXT as discussed above. Can't really commit this at the attribute layer, until we do something about reducing the event cost. Otherwise we'd be doubling events for every attribute change.

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 09/19/12
  • milestone changed from 3.NEXT to BACKLOG

Moving from 3.NEXT to BACKLOG.

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 08/22/13
  • owner changed from Satyen Desai to Jenny Donnelly
  • status changed from accepted to assigned