Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applies micro-optimizations and removes obsolete settings #5591

Merged
merged 12 commits into from
Sep 23, 2019

Conversation

sorvell
Copy link
Contributor

@sorvell sorvell commented Sep 12, 2019

Applies micro-optimizations that were found to improve element creation benchmarks by 5-10%, and removes obsolete settings:

  • Removed legacyNoBatch and legacyNotifyOrder settings.
  • dom-if/repeat: dom-change and renderedCount no longer fire with legacyOptimizations set.
  • LegacyElementMixin: isAttached now set before calling connectedCallback so it is batched with initial rendering.
  • PropertiesChanged: property accessor code now inlined for efficiency rather than calling _get/_setProperty. The __dataCounter tracking flag has been moved here to avoid the need to override _flushProperties in PropertyEffects.
  • PropertyEffects: inlined runEffectsForProperty into runEffects for efficiency. Removed wrapping around sending data events.
  • async: In the microtask scheduler, now only provoke a DOM mutation if needed.

Applies micro-optimizations that were found to improve element creation benchmarks by 5-10%, and removes obsolete settings:

* Removed `legacyNoBatch` and `legacyNotifyOrder` settings.
* dom-if/repeat: `dom-change` and `renderedCount` no longer fire with `legacyOptimizations` set.
* legacy-element-mixin: `isAttached` now set before calling `connectedCallback` so it is batched with initial rendering.
* `PropertiesChanged`: property accessor code now inlined for efficiency rather than calling `_get/_setProperty`. The `__dataCounter` tracking flag has been moved here to avoid the need to override `_flushProperties` in `PropertyEffects`.
* `PropertyEffects`: inlined `runEffectsForProperty` into `runEffects` for efficiency. Removed wrapping around sending data events.
* `async`: In the microtask scheduler, now only provoke a DOM mutation if needed.
@kevinpschaaf
Copy link
Member

Based on in-the-wild code relying on isAttached to be set after all observers have run, probably need to take out the isAttached change.

Copy link
Member

@dfreedm dfreedm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for travis changes

Copy link
Member

@kevinpschaaf kevinpschaaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, pending CI & TAP

…redItemCount on that.

Matches Polymer 1 setting for better backward compatibility.
This was causing a number of rendering tests to fail. Needs investigation, but possibly because wrapping calls ShadyDOM.flush, and this alters distribution timing which some tests may have inadvertently relied on.
lib/utils/settings.js Outdated Show resolved Hide resolved
@kevinpschaaf kevinpschaaf merged commit 6c3bb48 into legacy-undefined-noBatch Sep 23, 2019
@kevinpschaaf kevinpschaaf deleted the legach-undefined-noBatch-opt branch September 23, 2019 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants