Ticket #2530190 (closed defect)

Reporter


Satyen Desai
Opened: 04/28/11
Last modified: 05/10/12
Status: closed
Type: defect
Resolution: fixed

Owner


Satyen Desai
Target Release: 3.5.0
Priority: P2 (high)
Summary: Lighter component infrastructure
Description:

Need to reduce the kweight of Attribute, Base for use on the mobile stack.

KWeight:

K1) attribute-light: attribute-base without Events or "." support: Provides get/set interface, getter/setter/validator, constructor(userVals) support (USEFUL?) - 2 days
K2) Extract attribute-extras (lesser used methods - resetAttrs, modifyAttrs etc.) - 0.5 days
K3) Base - Revisit base-base implementation, for minor kweight savings. Can't think of anything to break out - 2 days
K4) Clean up OOTB meta-data (base -> base-base, attribute -> attribute-base) - 0.1 days

Type: defect Observed in Version: development master
Component: Base Severity: S3 (normal)
Assigned To: Satyen Desai Target Release: 3.5.0
Location: Library Code Priority: P2 (high)
Tags: Relates To:
Browsers: N/A
URL:
Test Information:

Change History

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/26/11
  • estimated changed from 0 to 6
  • sprint changed to sprint 2

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/26/11
  • estimated changed from 6 to 8

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/26/11
  • estimated changed from 8 to 8.5

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/26/11

If anyone listening has feedback on whether K1 is the ideal set of features to extract into an attribute-light, I'd be interested (as well as any other feedback of course)

Ryan Grove

YUI Developer

  • Username: rgrove
  • GitHub: rgrove
Posted: 05/26/11

I really like the idea of an attribute-base without modifyAttrs, etc., but I think it should still include change events (that seems like pretty fundamental functionality for attributes). That's essentially the minimum set of functionality I need in all of my modules.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/26/11
  • component changed from Widget to Base
  • owner changed from Satyen Desai to Satyen Desai
  • status changed from accepted to assigned

Just reclassifying the bug to focus on Base/Attribute. We can start another discussion for Widget and link it in - but that's going to be a separate problem space (optimizing DOM interactions, render as string etc.)

--

Regarding events...

Events have always been *THE* key value-add of Attribute in my mind (over dot properties).

That said, pulling out events is the only big chunk of both runtime and kweight cost which breaks out, and I've heard from folks who want the get/set interface without events (e.g. something Tripp is doing for markers in Charts). Hence wanted to make sure we capture use-cases and make the decision explicitly (one way or the other). If the overwhelming feedback is "I'd never use attribute without events" then it'll stay in there.

Tripp Bridges

YUI Developer

  • Username: tripp
  • GitHub: tripp
Posted: 05/26/11
  • component changed from Base to Widget
  • owner changed from Satyen Desai to Satyen Desai

I would like an optimized solution for batching multiple instances.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/26/11
  • component changed from Widget to Base
  • estimated changed from 8.5 to 4.6
  • owner changed from Satyen Desai to Satyen Desai
  • status changed from accepted to assigned

Split out kweight/runtime tasks

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 05/27/11
  • sprint changed from sprint 2
  • status changed from assigned to accepted

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 06/23/11
  • sprint changed to sprint 3

Should be relatively low risk for Sprint 3, but if we want to be strict about low level changes, I can move it out.

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 06/27/11
  • milestone changed from 3.4.0 to 3.5.0

Moving to backlog for 3.5.0.

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 06/27/11
  • sprint changed from sprint 3

Oops, need to also remove sprint.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 11/7/11
  • sprint changed to sprint 1

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 11/7/11
  • remaining changed from 0 to 4.6

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 11/9/11
  • priority changed from P3 (normal) to P2 (high)

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/6/11

This is what's in my head currently, and what I'll start executing against over the next couple of days (may not be done for PR1 - may overflow into early PR2)

Let me know if you see anything major you'd like to change.

GOALS


  • Provide a lighter version of the attribute interface, which doesn't fire change events [ currently key for performance - Node, Graphics are use cases. Alternative is to optimize Event ].
  • Provide a lighter version of the attribute interface, which doesn't provide the 20% usage.
  • Allow users to optionally re-augment events and/or the 20% usage to their attribute light based/augmented classes.
  • Provide a BaseCore and then finally a WidgetCore which can be driven by the lighter AttributeCore, but upgrade to the current Widget/Base/Attribute [ devil in the details really ].

"CLASSES"


  • Y.AttributeCore
    • Augmentable/Instantiable Attribute Provider without events or 20% API. Basically setter/getter/validator/readOnly/writeOnce - is this too much?
  • Y.AttributeEvents
    • Augmentable Attribute Event Provider
  • Y.AttributeExtras
    • Augmentable Attribute Extras 20% API [ name up for debate ]
  • Y.BaseCore = BaseBase + Y.AttributeCore
    • BaseBase used for illustration only - conveys that I'll take today's Base, rip out the Attribute stuff, and re-augment what's left with AttributeCore. There won't be a public Y.BaseBase
  • Y.Base = Y.BaseCore + Y.AttributeEvents + Y.AttributeExtras
    • Today's Base

AttributeExtras (20% usage)


  • modifyAttrs
  • removeAttr
  • reset
  • _getAttrCfg
  • . (dot) support
  • lazyAdd (maybe, will need to test perf)

AttributeCore


  • get
  • set
  • setAttrs
  • getAttrs
  • addAttrs/Attr
    • default/init value
    • out-of-order handling for addAttrs (attribute A setter asks for attribute B value, before B has been "added")
  • static ATTRS handling (protecting static cfgs/values)

Some things which I was thinking about removing from core, but decided to leave in (debatable?)

  • readOnly
  • writeOnce
  • validator
  • valueFn

I don't believe there's too much run-time/kweight cost to these features but we can revisit if required. My main concern is creating too confusing a mix/match picture for the relative savings achieved.

Attribute Core may decide to change the way it stores internal attribute state for performance. _conf/_state is currently private.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/6/11

Another piece I may break out - Y.AttributeStateProxy - the ability to store state somewhere other than the internal _state object. This is used by Node, where it passes certain attributes through to the domNode, and certain attributes it stores in _state.

Eric Ferraiuolo

YUI Developer

  • Username: ericf
  • GitHub: ericf
Posted: 12/8/11

I like the idea of the change to moving the 20% out into separate, mixable, feature modules. Ideally we want to drop the "Core" suffix off of those modules, and make them the main thing the other components of the library are built on. The parts that need the extra 20% features could mix them in as needed.

The, "Should I use Base, or Widget?" question people always ask will be complicated by the existence of BaseBase, and BaseCore. Would the plan be to make these adjustments going forward, making the ligher versions, the standard?

I always get confused on this, but shouldn't the classes with the "Core" suffix swap with the ones with the "Base" suffix? i.e., Y.BaseCore would be the guts, Y.BaseBase would be the thing people instantiate.

Another thing, should we use namespacing here, Y.Attribute.Core, Y.Base.Core and Y.Base.Base?
Luke tends to prefer this, maybe he can chime in; and whatever we choose, it should be consistent throughout the library.

Luke Smith

YUI Contributor

  • Username: lsmith
  • GitHub: lsmith
Posted: 12/8/11

I do prefer the namespacing. If there's an instantiable class that represents the 80% use case, it should be the one on Y, and edge case classes under the namespace. I agree with @ericf that "Core" and "Base" is bad naming for the class that serves the 80%. If a trim subset class is less likely to be consumed than a superset class, the superset class should be Y.TheClass, and the trim subset Y.TheClass.Base* (or Core if necessary). Same for the reverse: if the subset is more trafficked, it should be Y.TheClass and the superset Y.TheClass.Full (or some other feature suggestive name). I'm on the fence about namespaced vs multiple Y.Classes if there's a reasonable amount of consumption of both subset and superset, but leaning toward namespacing.

I think pulling event is likely the fastest way to see run time performance improvements for this set of use cases, but I question their commonality. Is there really so much Attribute code to support firing the change events? If it's all nickel and dime trimming, then I would suggest dropping the features you previously dropped, then restored. I have also always considered the change events to be the big selling point to Attribute.

I'm curious what features the folks asking for an eventless Attribute are using. I can understand wanting a class that shares the get()/set() API that is common across the lib, but isn't saddled with the overhead of event firing lifecycle for set() IFF there are compelling features behind get() and set() that make it preferable to using simple objects+properties. There is value in API consistency and future flexibility of enforcing the common APIs, but I would feel more comfortable if the pitch included the other configs you pulled then restored. If there is strong demand for eventless attributes with API features, I agree with your proposal.

However, I also want a version of Attribute that includes change events, but doesn't include anything beyond value and validator. I think this would also serve a large number of use cases, and at least leaves the door open for more performance boost from an event refactor. The challenge with such an event refactor, IMO, would be from bubbling, which I've seen becoming increasingly popular. It's potentially easy to make fire() a noop until there are subscriptions, but once addTarget() is called, the no-subs == noop assumption can't be made, so more invasive performance improvements will need to be made. There's more to this, but I suspect you've profiled the methods and found the impact of the event lifecycle to be far greater than any of the setter/getter/readOnly/etc features. I wonder if event stays, if the marginal k weight/runtime improvement from dropping other features would be worthwhile. Maybe better to focus on event in this case.

  • Y.TheClass.Base is only appropriate if there is a direct/clean inheritance relationship. That is, if the Base implementation is augmented for TheClass. I'm not opposed to alternate implementations that serve the same public API contract being namespaced under TheClass, but they should not be named Base or Core. Beyond the scope of this ticket, but I couldn't help qualifying my statement.

Evan Goer

YUI Contributor

Posted: 12/8/11

Take my comments with a big grain of salt, but FWIW I agree with Luke that change events are the big selling point for Attribute.

To me, attributes mean state management, and "management" means "control" and "monitoring". value, validator, valueFn, etc. give you control, and change events give you monitoring. (And also some control.)

Anyway, take all those things away, and you're left with just getters and setters, which aren't terribly exciting. People can just use properties. And if necessary, write getters and setters for them.

On k-weight -- if you haven't already, it might be worth spidering through YUI core and YUI gallery code to count how many attributes there are, and which features are actually being used in the wild.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/8/11

Regarding Event

The major chunk of run-time savings comes from dropping event, as previously profiled

For example, just short-circuiting the single fire("init") event to invoke the defaultFn() directly in Base (ala no listeners/no bubble listeners), results in a 50% savings in the time required to instantiate a Base based object.

That has been why I've been pushing for getting publish() and fire() as close to no-ops as we can get them in the case where there's no bubbling and no subscribers, to facilitate the above type of savings for attribute changes - in which case we wouldn't need an event-less Attribute, and maybe could do without a WidgetCore.

However since that seems like a longer term project, AttributeCore/BaseCore/WidgetCore is the fallback solution.

I mention my take on Attribute Events in the previous comments. Quote:

> "Events have always been *THE* key value-add of Attribute in my mind (over dot properties). However ..."

> "Provide a lighter version of the attribute interface, which doesn't fire change events [ currently key for performance - Node, Graphics are use cases. Alternative is to optimize Event ]."

Regarding Non-Event Usage

We have developers who want to:

a) Establish the get/set interface and move up to events/extra value optionally.
b) Want to use the centralized state management which setter/getter/validator/valueFn logic provides.
c) Want to use the default attribute handling (the ATTRS support).

As mentioned above, Charts is one such use case, where Tripp is rolling his own AttributeLite. That is what AttributeCore is trying to replace. It may also form the basis for a WidgetCore (aka Node Plugin), which doesn't have Attribute change events, if there's a migration path up from WidgetCore to Widget. Node is another potential use case, where Matt is rolling his own AttributeLite.

In terms of both kweight and runtime cost, readOnly/writeOnce and the other configs are not big chunks, relative to event - they boil down to the "if" check + "function hop" into State. Hence the decision not to split them out from get/set/setter/getter support and keep them in core.

Regarding Core/Base

Y.Attribute and Y.Base need to stay as they are for backwards compatibility

There is no Y.AttributeBase or Y.BaseBase, so I'm not sure what the "Core" vs "Base" reference is about.

Only:

Y.Attribute [ current, majority usage ]
Y.Base [ current, majority usage ]

Y.AttributeCore [ lightest/fastest attribute usage ]
Y.BaseCore [ lightest/fastest base usage ]

With:

Y.AttributeEvents
Y.AttributeExtras [ or better name ]

As augmentable pieces.

Regarding Namespacing

I personally find the namespacing fine, even preferable (and argued for it during the early YUI 3 conversation). I like the idea of grouping implementation classes.

However there's no real technical or usability benefit to it, unless it drives infrastructure which iterates classes/impls hanging off a "namespace" - which is where it does add value. The end user is not going to be iterating Y to see what hangs off it. They're also not going to be iterating Y.Foo to see what hangs off it. Exploring things in the console is potentially the only real use case I see it driving currently, where Y.AttributeCore and Y.Attribute.Core really amount to the same thing.

On the flip side, if we're talking about consistency, non-namespaced is probably the predominant format. Additionally namespacing probably adds technical and documentation overhead.

I'm fine either way, but sticking with the majority case for now. We can flip if required.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/8/11
  • completed changed from 0 to 2
  • remaining changed from 4.6 to 3.5

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/12/11
  • completed changed from 2 to 1

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/12/11

Have:

Y.AttriubteCore
Y.AttributeEvents
Y.AttributeExtras

(namespacing subject to change as mentioned above)

Broken out, and Y.Attribute put back together again and passing all attribute, attribute-core, base and widget unit tests.

Also changed the structure of State's internal data property to store [name][property] instead of [property][name] so we can getAll() more effectively for "name" centric operations.

Only part I'm not too happy with is the fact that I need an overwrite "true" in Y.augment(SomeAttributeCoreImpl, AttributeEvents, true).

Luke Smith

YUI Contributor

  • Username: lsmith
  • GitHub: lsmith
Posted: 12/12/11

"Exploring things in the console is potentially the only real use case I see it driving currently, where Y.AttributeCore and Y.Attribute.Core really amount to the same thing."

Agreed. It's mainly an academic argument. Y is a namespace for the public API of the lib. I like to keep the API trim and focused, and "other stuff" tucked away in underscore properties or namespaces.

"On the flip side, if we're talking about consistency, non-namespaced is probably the predominant format."

I'm not a fan of status quo == convention. Doing something one way because it's been done that way to date is not a compelling argument if the way it has been done was not for a good reason. If there's a good reason for an alternative way, it should win. We can employ the deprecation strategy during migration.

"Additionally namespacing probably adds technical and documentation overhead."

Agreed. I suspect we'd run into trouble with yuidoc and the API docs either wanting to list a bunch of classes named Core or Base, or clobbering or merging the class def APIs. That would be an issue with yuidoc that should be addressed anyway, but shouldn't be a reason not to do something that we agree makes sense architecturally (assuming this were the case). The technical overhead argument can be viewed both ways. If Y hosts the public API methods and the 80% versions of instantiable classes, it would be easier for us to document an 80% library-wide API. We don't have such a document today (the individual user guides are the closest thing). I do think such a document would be valuable, though.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/17/11
  • status changed from accepted to checkedin

commit 496e4afa29c8a6fae3c6ddd03f0f7f42f968c3ad
Author: Satyen Desai sdesai@yahoo-inc.com
Date: Fri Dec 16 00:02:50 2011 -0800

Added Y.BaseCore, which is core Base functionality, but which uses Y.AttributeCore instead of Y.Attribute. Y.Base is now built by augmenting Y.BaseCore and Y.Attribute

commit fdaf522f78615dc3145b1771f6f8daba2df06bb6
Author: Satyen Desai sdesai@yahoo-inc.com
Date: Tue Dec 13 12:22:48 2011 -0800

Broke up Y.Attribute into:

Y.AttributeCore (Attribute without Events, or lesser used features)
Y.AttributeEvents (AttributeEvent support)
Y.AttributeExtras (Attribute lesser used features)

Y.Attribute remains 100% backwards compatible.

Attribute/Base/Widget/Plugin/App/Node/WidgetParentChild tests all pass.

Y.State's internal data structure was changed.

If you're accessing the private _state.data's fields directly, you'll
need to change code.

HISTORY.md has details.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/17/11
  • completed changed from 1 to 5
  • remaining changed from 3.5 to 0

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 12/20/11

Some initial benchmarks Base vs BaseCore (committed to src/base/tests/benchmark)

MyBaseCore with 20 varied attributes x 5,049 ops/sec ±1.23% (55 runs sampled)
MyBaseCore with 10 simple value attributes x 10,508 ops/sec ±1.16% (55 runs sampled)
MyBaseCore x 34,532 ops/sec ±1.28% (53 runs sampled)
BaseCore x 34,126 ops/sec ±2.67% (52 runs sampled)

MyBase with 20 varied attributes x 2,832 ops/sec ±1.29% (53 runs sampled)
MyBase with 10 simple value attributes x 4,567 ops/sec ±1.33% (54 runs sampled)
MyBase x 6,517 ops/sec ±2.82% (51 runs sampled)
Base x 6,519 ops/sec ±1.88% (32 runs sampled)

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 05/10/12
  • resolution changed to fixed

checkedin -> closed

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 05/10/12
  • status changed from checkedin to closed

checkedin -> closed