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

perf(platform-server): use shared `DomElementSchemaRegistry` instance (#28150) #28151

Closed
wants to merge 1 commit into
base: master
from

Conversation

@bmeurer
Copy link
Contributor

bmeurer commented Jan 15, 2019

Right now the ServerRendererFactory2 creates a new instance of the DomElementSchemaRegistry for each and every request, which is quite costly (for the Tour of Heroes SSR example this takes around 30% of the overall execution time). Since the schema is never modified, but only used in a read-only fashion, it should be possible to re-use a single instance instead.

Naive performance testing with 100 concurrent connections and 1000 requests in total shows an approximate 33% improvement in Req/Sec on the Tour of Heroes SSR example (can be found at bmeurer/angular-tour-of-heroes).

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

A new DomElementSchemaRegistry instance is created for each and every ServerRendererFactory2.

Issue Number: #28150

What is the new behavior?

A global DomElementSchemaRegistry instance shared by all ServerRendererFactory2 instances.

Does this PR introduce a breaking change?

  • Yes
  • No

@bmeurer bmeurer requested a review from angular/fw-server as a code owner Jan 15, 2019

@googlebot googlebot added the cla: yes label Jan 15, 2019

@ngbot ngbot bot added this to the needsTriage milestone Jan 15, 2019

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor

AndrewKushnir commented Jan 15, 2019

perf(platform-server): use shared `DomElementSchemaRegistry` instance (
…#28150)

Right now the `ServerRendererFactory2` creates a new instance of the
`DomElementSchemaRegistry` for each and every request, which is quite
costly (for the Tour of Heroes SSR this takes around **30%** of the
overall execution time). Since the schema is never modified, but only
used in a read-only fashion, it should be possible to re-use a single
instance instead.

Naive performance testing with 100 concurrent connections and 1000
requests in total shows an approximate **33%** improvement in Req/Sec
on the Tour of Heroes SSR example.

PR Close #28150

@bmeurer bmeurer force-pushed the bmeurer:singleton-DomElementSchemaRegistry branch from 559675d to c00d6d3 Jan 15, 2019

@bmeurer

This comment has been minimized.

Copy link
Contributor Author

bmeurer commented Jan 15, 2019

Actually after removing the artificial default 500ms delay for the InMemoryDataService used by the tutorial, I see a ~33% increase in Req/Sec now.

AndrewKushnir added a commit that referenced this pull request Jan 15, 2019

perf(platform-server): use shared `DomElementSchemaRegistry` instance (
…#28150) (#28151)

Right now the `ServerRendererFactory2` creates a new instance of the
`DomElementSchemaRegistry` for each and every request, which is quite
costly (for the Tour of Heroes SSR this takes around **30%** of the
overall execution time). Since the schema is never modified, but only
used in a read-only fashion, it should be possible to re-use a single
instance instead.

Naive performance testing with 100 concurrent connections and 1000
requests in total shows an approximate **33%** improvement in Req/Sec
on the Tour of Heroes SSR example.

PR Close #28150

PR Close #28151

@bmeurer bmeurer deleted the bmeurer:singleton-DomElementSchemaRegistry branch Jan 15, 2019

AndrewKushnir added a commit that referenced this pull request Jan 16, 2019

perf(platform-server): use shared `DomElementSchemaRegistry` instance (
…#28150) (#28151)

Right now the `ServerRendererFactory2` creates a new instance of the
`DomElementSchemaRegistry` for each and every request, which is quite
costly (for the Tour of Heroes SSR this takes around **30%** of the
overall execution time). Since the schema is never modified, but only
used in a read-only fashion, it should be possible to re-use a single
instance instead.

Naive performance testing with 100 concurrent connections and 1000
requests in total shows an approximate **33%** improvement in Req/Sec
on the Tour of Heroes SSR example.

PR Close #28150

PR Close #28151

wKoza added a commit to wKoza/angular that referenced this pull request Jan 18, 2019

perf(platform-server): use shared `DomElementSchemaRegistry` instance (
…angular#28150) (angular#28151)

Right now the `ServerRendererFactory2` creates a new instance of the
`DomElementSchemaRegistry` for each and every request, which is quite
costly (for the Tour of Heroes SSR this takes around **30%** of the
overall execution time). Since the schema is never modified, but only
used in a read-only fashion, it should be possible to re-use a single
instance instead.

Naive performance testing with 100 concurrent connections and 1000
requests in total shows an approximate **33%** improvement in Req/Sec
on the Tour of Heroes SSR example.

PR Close angular#28150

PR Close angular#28151

wKoza added a commit to wKoza/angular that referenced this pull request Jan 18, 2019

perf(platform-server): use shared `DomElementSchemaRegistry` instance (
…angular#28150) (angular#28151)

Right now the `ServerRendererFactory2` creates a new instance of the
`DomElementSchemaRegistry` for each and every request, which is quite
costly (for the Tour of Heroes SSR this takes around **30%** of the
overall execution time). Since the schema is never modified, but only
used in a read-only fashion, it should be possible to re-use a single
instance instead.

Naive performance testing with 100 concurrent connections and 1000
requests in total shows an approximate **33%** improvement in Req/Sec
on the Tour of Heroes SSR example.

PR Close angular#28150

PR Close angular#28151
@naveedahmed1

This comment has been minimized.

Copy link

naveedahmed1 commented Jan 22, 2019

Great job @bmeurer 👍

ngfelixl added a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019

perf(platform-server): use shared `DomElementSchemaRegistry` instance (
…angular#28150) (angular#28151)

Right now the `ServerRendererFactory2` creates a new instance of the
`DomElementSchemaRegistry` for each and every request, which is quite
costly (for the Tour of Heroes SSR this takes around **30%** of the
overall execution time). Since the schema is never modified, but only
used in a read-only fashion, it should be possible to re-use a single
instance instead.

Naive performance testing with 100 concurrent connections and 1000
requests in total shows an approximate **33%** improvement in Req/Sec
on the Tour of Heroes SSR example.

PR Close angular#28150

PR Close angular#28151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.