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

fix: error when 2 perf markers are stacked #865

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

jodarove
Copy link
Contributor

@jodarove jodarove commented Nov 29, 2018

Details

We have 3 prod perf marks today, init, hydrate and rehydration queue.

When 2 marks are stacked, for example, creating a component in the constructor of another will stack 2 init marks, once the second marker is measured, we need to clean the marks and measures and will clear the first one, and when the measure for that marker is done, it throws.

Those cases are for init and hydrate, the rehydration queue measure is fine, we dont want to do it per component.

This pr does 2 things:
1- removes the init measure, cause we dont have a vm, and by the time is created in create component, it does not make sense to measure.
2- makes the marks for hydrate vm dependant, but the measure name is general, to pair it with the rehydration queue measure.

Does this PR introduce a breaking change?

  • Yes
  • No

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 0f41231 | Target commit: 7496480

lwc-engine-benchmark

table-append-1k metric base(0f41231) target(7496480) trend
benchmark-table/append/1k duration 149.75 (±3.95 ms) 152.60 (±4.55 ms) +2.8ms (1.9%) 👎
table-clear-1k metric base(0f41231) target(7496480) trend
benchmark-table/clear/1k duration 6.10 (±0.30 ms) 6.50 (±0.50 ms) +0.4ms (6.6%) 👎
table-create-10k metric base(0f41231) target(7496480) trend
benchmark-table/create/10k duration 872.40 (±7.60 ms) 874.30 (±5.85 ms) +1.9ms (0.2%) 👌
table-create-1k metric base(0f41231) target(7496480) trend
benchmark-table/create/1k duration 117.65 (±3.45 ms) 120.35 (±3.55 ms) +2.7ms (2.3%) 👎
table-update-10th-1k metric base(0f41231) target(7496480) trend
benchmark-table/update-10th/1k duration 75.90 (±2.50 ms) 77.60 (±2.20 ms) +1.7ms (2.2%) 👎
tablecmp-append-1k metric base(0f41231) target(7496480) trend
benchmark-table-component/append/1k duration 251.15 (±4.85 ms) 250.40 (±6.60 ms) -0.7ms (0.3%) 👌
tablecmp-clear-1k metric base(0f41231) target(7496480) trend
benchmark-table-component/clear/1k duration 12.15 (±1.65 ms) 12.40 (±2.00 ms) +0.3ms (2.1%) 👌
tablecmp-create-10k metric base(0f41231) target(7496480) trend
benchmark-table-component/create/10k duration 1712.70 (±14.05 ms) 1748.50 (±12.05 ms) +35.8ms (2.1%) 👎
tablecmp-create-1k metric base(0f41231) target(7496480) trend
benchmark-table-component/create/1k duration 210.50 (±5.25 ms) 212.55 (±5.05 ms) +2.1ms (1.0%) 👌
tablecmp-update-10th-1k metric base(0f41231) target(7496480) trend
benchmark-table-component/update-10th/1k duration 71.50 (±4.55 ms) 71.45 (±4.85 ms) -0.1ms (0.1%) 👌
wc-append-1k metric base(0f41231) target(7496480) trend
benchmark-table-wc/append/1k duration 259.80 (±8.25 ms) 259.40 (±8.85 ms) -0.4ms (0.2%) 👌
wc-clear-1k metric base(0f41231) target(7496480) trend
benchmark-table-wc/clear/1k duration 22.50 (±2.55 ms) 22.40 (±2.35 ms) -0.1ms (0.4%) 👌
wc-create-10k metric base(0f41231) target(7496480) trend
benchmark-table-wc/create/10k duration 1829.80 (±31.40 ms) 1846.45 (±45.50 ms) +16.6ms (0.9%) 👌
wc-create-1k metric base(0f41231) target(7496480) trend
benchmark-table-wc/create/1k duration 224.00 (±6.25 ms) 225.40 (±5.50 ms) +1.4ms (0.6%) 👌
wc-update-10th-1k metric base(0f41231) target(7496480) trend
benchmark-table-wc/update-10th/1k duration 74.25 (±4.45 ms) 72.80 (±5.80 ms) -1.4ms (2.0%) 👌

We have 3 prod markers today, init, hydrate and rehydration queue.

When 2 markers are stacked, for example, creating a component in the constructor of another will stack 2 init marks, once the second marker is measured, we need to clean the marks and measures and will clear the first one, and when the measure for that marker is done, it throws.

Those cases are for init and hydrate, the rehydration queue measure is fine, we dont want to do it per component.

This pr does 2 main things:
1- removes the init measure, cause we dont have a vm, and by the time is created in create component, it does not make sense to measure.
2- makes the marks for hydrate vm dependant, but the measure name is general, to pair it with the rehydration queue measure.
@jodarove jodarove force-pushed the jodarove/fix-stacked-measures branch from 7496480 to fbb0836 Compare November 30, 2018 01:43
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 3e38821 | Target commit: fbb0836

lwc-engine-benchmark

table-append-1k metric base(3e38821) target(fbb0836) trend
benchmark-table/append/1k duration 149.15 (±5.10 ms) 150.35 (±4.10 ms) +1.2ms (0.8%) 👌
table-clear-1k metric base(3e38821) target(fbb0836) trend
benchmark-table/clear/1k duration 6.30 (±0.50 ms) 6.60 (±0.40 ms) +0.3ms (4.8%) 👌
table-create-10k metric base(3e38821) target(fbb0836) trend
benchmark-table/create/10k duration 881.45 (±7.00 ms) 881.85 (±8.15 ms) +0.4ms (0.0%) 👌
table-create-1k metric base(3e38821) target(fbb0836) trend
benchmark-table/create/1k duration 119.30 (±2.60 ms) 119.15 (±3.05 ms) -0.1ms (0.1%) 👌
table-update-10th-1k metric base(3e38821) target(fbb0836) trend
benchmark-table/update-10th/1k duration 78.60 (±3.35 ms) 78.40 (±4.40 ms) -0.2ms (0.3%) 👌
tablecmp-append-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-component/append/1k duration 257.45 (±9.20 ms) 256.15 (±4.75 ms) -1.3ms (0.5%) 👌
tablecmp-clear-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-component/clear/1k duration 12.90 (±1.40 ms) 12.10 (±1.65 ms) -0.8ms (6.2%) 👌
tablecmp-create-10k metric base(3e38821) target(fbb0836) trend
benchmark-table-component/create/10k duration 1748.25 (±12.80 ms) 1705.40 (±11.15 ms) -42.8ms (2.5%) 👍
tablecmp-create-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-component/create/1k duration 207.00 (±7.20 ms) 213.00 (±5.20 ms) +6.0ms (2.9%) 👎
tablecmp-update-10th-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-component/update-10th/1k duration 72.25 (±4.65 ms) 70.05 (±3.65 ms) -2.2ms (3.0%) 👌
wc-append-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-wc/append/1k duration 253.25 (±8.00 ms) 255.10 (±8.05 ms) +1.8ms (0.7%) 👌
wc-clear-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-wc/clear/1k duration 23.05 (±2.85 ms) 22.95 (±3.05 ms) -0.1ms (0.4%) 👌
wc-create-10k metric base(3e38821) target(fbb0836) trend
benchmark-table-wc/create/10k duration 1839.35 (±30.40 ms) 1826.65 (±31.35 ms) -12.7ms (0.7%) 👌
wc-create-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-wc/create/1k duration 223.30 (±5.95 ms) 225.15 (±5.70 ms) +1.8ms (0.8%) 👌
wc-update-10th-1k metric base(3e38821) target(fbb0836) trend
benchmark-table-wc/update-10th/1k duration 74.70 (±4.25 ms) 72.25 (±4.20 ms) -2.4ms (3.3%) 👍

@caridy
Copy link
Contributor

caridy commented Nov 30, 2018

I don't understand this PR :(

Copy link
Contributor

@caridy caridy left a comment

Choose a reason for hiding this comment

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

I will defer this to @diervo

@diervo diervo merged commit 1bce3c7 into master Dec 12, 2018
@diervo diervo deleted the jodarove/fix-stacked-measures branch December 12, 2018 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants