Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

perf(renderComponent): avoid multiple style function calls #1957

Merged
merged 3 commits into from
Sep 23, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Sep 23, 2019

馃摉 Story around

We need to compute classes and styles to pass them to components and pass down to slots. However, we previously we did the same amount of work twice:

// `mergedStyles` is an object with style functions
const resolvedStyles: ComponentSlotStylesPrepared = resolveStyles(mergedStyles, styleParam)
const classes: ComponentSlotClasses = getClasses(renderer, mergedStyles, styleParam)

resolveStyles() and mergedStyles() resolved styles twice and means that all style functions were called twice. With these changes getClasses() will use already resolved styles to avoid double calls.

馃悗 Measures

cross-env PERF=true gulp perf --filter *Chat.perf.tsx --times=100

Performance diff is around 5.80%.

Before

Example Avg Median
Chat.perf.tsx 482.15 480.24
Chat.perf.tsx 483.21 482.86
Chat.perf.tsx 484.41 483
Total 483.25

After

Example Avg Median
Chat.perf.tsx 454.69 452.21
Chat.perf.tsx 455.29 453.38
Chat.perf.tsx 457.94 454.18
Total 455.97

@codecov
Copy link

codecov bot commented Sep 23, 2019

Codecov Report

Merging #1957 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1957      +/-   ##
==========================================
- Coverage   70.31%   70.26%   -0.05%     
==========================================
  Files         894      893       -1     
  Lines        7895     7891       -4     
  Branches     2287     2309      +22     
==========================================
- Hits         5551     5545       -6     
- Misses       2331     2333       +2     
  Partials       13       13
Impacted Files Coverage 螖
packages/react/src/components/Design/Design.tsx 22.22% <酶> (酶) 猬嗭笍
packages/react/src/lib/renderComponent.tsx 82.53% <100%> (-2.21%) 猬囷笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update aed24a7...6cef4a7. Read the comment docs.

### Documentation
- Remove Usage tab @lucivpav ([#1948](https://github.com/stardust-ui/react/pull/1948))
- Put props on a single page, fix props links @lucivpav ([#1892](https://github.com/stardust-ui/react/pull/1892))

### Documentation
Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, duplicate

Copy link
Member

@miroslavstastny miroslavstastny left a comment

Choose a reason for hiding this comment

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

馃憤

@layershifter layershifter merged commit 267a8bc into master Sep 23, 2019
@layershifter layershifter deleted the perf/merge-one-time branch September 23, 2019 12:41
layershifter added a commit that referenced this pull request Sep 26, 2019
layershifter added a commit that referenced this pull request Sep 26, 2019
layershifter added a commit that referenced this pull request Sep 26, 2019
* perf(renderComponent): avoid multiple style function calls (#1957)

(cherry picked from commit 267a8bc)

* fix(tooltip): Make tooltip visible for screen reader  (#1942)

(cherry picked from commit 8295ce9)

* docs: add version header to CHANGELOG

* docs: changelog fix header comment

* docs: update changelog
layershifter added a commit that referenced this pull request Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants