Skip to content

Conversation

@marcelltoth
Copy link
Contributor

Times in parenthesis are mounting times of the 2x-stress schema story in Storybook.

  • Bumps @stoplightio/json-schema-merge-allof (1660ms -> 1590ms)
  • Removes trivial mosaic components from the performance-critical rendering paths: SchemaRow and beyond (1590ms -> 850ms)

The problem with mosaic components is that for almost 0 gain (beyond type-checking of style-props), you render yet another element, and especially yet another forwardRef-ed element, both of which eat up performance as you scale.

@marcelltoth marcelltoth requested review from a team and P0lip March 18, 2021 13:14
@marcelltoth marcelltoth self-assigned this Mar 18, 2021
@marcelltoth marcelltoth requested a review from a team March 18, 2021 13:14
Copy link
Contributor

@P0lip P0lip left a comment

Choose a reason for hiding this comment

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

👍 perhaps someone from undefined-team should sign off the UI part of this PR.
It looks good to me, yet I shouldn't be the one deciding whether you're dropping certain mosaic components or not. However as an external person, I'd say it's a reasonable choice.

Copy link
Contributor

@mpodlasin mpodlasin left a comment

Choose a reason for hiding this comment

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

I wasn't able to detect any visual regressions, so I believe this is ok!

@marcelltoth marcelltoth merged commit 2d3e951 into beta Mar 19, 2021
@marcelltoth marcelltoth deleted the perf/improve branch March 19, 2021 11:43
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 4.0.0-beta.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@marbemac marbemac restored the perf/improve branch May 19, 2021 00:56
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@P0lip P0lip deleted the perf/improve branch June 24, 2021 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants