Improve SimpleCSSValuesImpl

[Harbs]

Currently SimpleCSSValuesImpl gets style values like so:

1. returns the value from the underlying style if it exists.
2. Walks up the parent tree and returns the parent value if it's inherited.*
3. I uses a lookup for either css class names or Class names to get assigned style info
4. The Class names are saved in compiled MXML and added to the SimpleCSSValuesImpl by a call to ValuesManager.valuesImpl.init(this) in any class which implements IMXMLDocument
5. If the object is a class, it checks the qualified name and uses that to look up the styles.
6. When it doesn't find the styles, it will walk up the super-tree and does the same for each of the super-classes.

This has multiple issues:

1. Getting styles can be fairly expensive. As mentioned by @estanglerbm https://lists.apache.org/thread/qb1x76ohszmllkflzrq3mg70gy20y6tv The style lookups can be made much more efficient.
2. Requiring lookups based on qualified class names requires keeping these names around in minified code for every class in the application.
3. There's no way to cache known values to make lookups easier. (related to 1)
4. The code is pretty convoluted and can be made cleaner.

Additonally there's some issues identified here: https://github.com/apache/royale-asjs/issues/641#issuecomment-573836029

An example of a compiled MXML lookup is this:

0,
1,
"org.apache.royale.core.Application",
function() {this["MozUserSelect"] = "none";
this["MsUserSelect"] = "none";
this["WebkitUserSelect"] = "none";
this["margin"] = 0.0;
this["padding"] = 0.0;
this["userSelect"] = "none"},
0,
1,
"org.apache.royale.html.Container",
function() {this["alignItems"] = "flex-start";
this["iBeadLayout"] = org.apache.royale.html.beads.layouts.BasicLayout;
this["iBeadView"] = org.apache.royale.html.beads.ContainerView;
this["iViewport"] = org.apache.royale.html.supportClasses.OverflowViewport;
this["iViewportModel"] = org.apache.royale.html.beads.models.ViewportModel},

Here's a potential improvement:

1. Instead of using "org.apache.royale.html.Container", the value can be written unquoted to refer to the class directly like we do for bead references. This will enable to compiler to minimize the references normally.
2. This means that you can't add the class names to a hash in the valuesImpl.
3. Instead, we can add to IStyleableObject setStyle() and getStyle() That would put the logic of saving the style information and retrieving it in the class itself.
4. setStyle() and getStyle() would be instance methods, but it would save the information to the class so it's written once.
5. Additonally, getStyle can cache info it gets from the super-class so subsequent lookups would not need to walk up the hierarchy again. **
6. We might have additional methods for saveCacheValue and getCacheValue for optimizing lookups when walking up parents if it's known that it's safe to do so. These values would need to be saved in instances for it to work. The right balance of memory usage and speed optimization would need to be found for this.

• It looks like there's an inefficiency in step 2 where it will walk up the tree multiple times if the value is not set anywhere up the tree. That's because both getValue and getInheritedValue is called recursively in getInheritedValue. The current parent should probably be saved as an instance variable to prevent having to walk up the tree many times.
• • need to figure out if caching the super-values might be an issue for modules.

[Harbs]

After thinking about this some more:

• It seems like instead of adding to IStyleableObject, we should add a new interface (ICSSStyleable) which inherits from IStyleableObject.
• I'm not sure if UIBase should implement ICSSStyleable or there should be a subclass of that which does.
• There should probably be 5 distinct methods in ICSSStyleable.
  1. saveStyle (or similar) would save a default for the class.
  2. findStyle would get a style from the defaults walking up the inheritance if necessary. The results should be cached so the walk would only happen once per style per class.
  3. findInheritedStyle would call find style up the parent chain until it finds a style. The parent chain results would not be cached by default. We'd need to figure out whether caching is ever a good idea.
  4. setStyle would apply a style to an instance. In the case of HTML, this should apply the change to the element's inline css.
  5. getStyle would get the directly applied style of the instance (if any).
• In MX/Spark setStyle and getStyle already have defined meanings. We'd need to figure out how/if that fits with the above. mx.core.UIComponent might need a different implementation. This might be a reason to make the implementation a subclass of UIBase.
• For many current uses of ValuesManager.valuesImpl.getValue(), there is no need to walk up the parent chain (such as finding default beads). For such cases simply calling findStyle() would be enough.
• It's worth researching whether we can/should have an option to avoid style lookups for cases where it's not needed (such as HTML elements where it's known that styling is applied by HTML CSS). This might provide significant performance improvements.
• It's worth researching how expensive it currently is to get an element's parent. It currently uses the DOM for HTML elements to get the DOM element's parent. The rule of thumb is that DOM operations are much more expensive than native JS. It might be worthwhile looking into whether keeping track of the parent is something that can improve performance. If yes, this would need to be done carefully to make sure that parent references don't get out of sync.

I'd love to hear others' thoughts on all of this.

[estanglerbm]

I haven't measured specifically, but a single DOM parentNode is pretty quick (it's not an expensive traversal and doesn't modify the DOM).

I think the recursion (classes + containers) of each getStyle() call is more the issue, in general, since getStyle() is called so much.

[aharui]

I'd recommend modifying a version of SimpleCSSValuesImpl so that it can log all calls to getStyle so we truly understand why it is being called. If 99% is for bead lookup, then optimize bead lookup. There shouldn't be many, if any, calls to getStyle in JS. Also, the parent shouldn't be involved if the style is not inheriting.