search

play-co / timestep

GNU General Public License v3.0
16 stars 27 forks source link

Refactoring of view backing layout extension #127

Closed bchevalier closed 7 years ago

bchevalier commented 7 years ago

Why

ViewBacking objects would always have layout properties attached to them even when unnecessary (In Everwing, about 80 views are using layout properties out of several thousand views).

I hypothesized that the way the ViewBackingExtension was written could stop the optimizer-compiler from properly optimizing a few methods (the updateGlobalTransform still was not down to the expected resource usage regarding its work load as mentioned in https://github.com/gameclosure/timestep/pull/123).

How

A LayoutViewBacking subclass of ViewBacking now replaces the ViewBackingExtension. Only views that have a layout instantiate a style using the LayoutViewBacking constructor (Other views simply have a style of type ViewBacking). This LayoutViewBacking extension was rewritten to avoid using the setProperty API.

Also made sure that some class properties are initialized in the constructor to limit the number of hidden classes used by methods of the rendering pipeline.

Note: No refactoring is need in the code base of existing games but in order to achieve it a _setLayout method that would trigger when updating the layout on a view style was added to View. This method looks like an elaborate hack, I think that ideally we would need to figure out a different way to create layouts.

Results

Overall it looks like a minor performance improvement (~5%) and would not expect a huge FPS boost from this PR.

Engine tick

The ticking of the engine appears ~15% faster though: Before

screen shot 2017-06-14 at 7 15 09 pm

After

screen shot 2017-06-16 at 6 49 11 pm

Chrome profile (macbook)

Before

chrome-after

After

screen shot 2017-06-17 at 2 37 59 pm

Safari profile (iPhone 7)

Before

screen shot 2017-06-14 at 4 07 10 pm

After

screen shot 2017-06-19 at 3 16 34 pm

Resource usage of the updateGlobalTransform has been divided by more than 3 on both Chrome and Safari. It now seems that this method is properly optimized to the expected level.

Note: On safari iOS it appears that the profiler is not working entirely properly . Self time = total time and idle time is non-existent but seems to be hiding under the drawElements profile. Under this assumption it seems that idle time increase by 4%, hinting at a 7% performance boost (from 56% non-idle time to 52%).

rogueSkib commented 7 years ago

This looks great, tested and found no issues.