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

Make it easy to add more stat types to work packet statistics #324

Merged
merged 16 commits into from
May 28, 2021

Conversation

caizixian
Copy link
Member

@caizixian caizixian commented May 18, 2021

This PR refactors work-packet level statistics collection.
Work packet execution time measuring was hardcoded. It's now abstracted through a WorkCounter trait interface.
This PR also makes #315 easier.

@caizixian caizixian requested a review from wenyuzhao May 18, 2021 09:04
@caizixian caizixian added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label May 18, 2021
@caizixian caizixian marked this pull request as ready for review May 18, 2021 14:26
Copy link
Member

@wenyuzhao wenyuzhao left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

As a maintainer of the code base, I have no idea what the code tries to achieve with no description in the PR and no comment in the code.

@caizixian caizixian requested a review from qinsoon May 20, 2021 12:33
@github-actions
Copy link

JikesRVM

NoGC (wrench-2021-05-20-Thu-231104)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 776.80 ±5.30 774.74 ±3.37 ⚠️ 1 removed 774.5 778.25 ±4.07 777.23 ±3.61 ⚠️ 1 removed 776.0 +0.19% +0.32%
fop 627.23 ±1.87 627.23 ±1.87 627.0 628.35 ±1.68 628.35 ±1.68 627.0 +0.18% +0.18%
luindex 2262.92 ±7.08 ⚠️ 1/40 failed 2262.92 ±7.08 2265.0 2271.85 ±6.47 2271.85 ±6.47 2272.5 +0.39% +0.39%

SemiSpace (wrench-2021-05-20-Thu-232302)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 637.88 ±12.07 637.88 ±12.07 618.5 651.25 ±12.80 651.25 ±12.80 632.0 +2.10% +2.10% 🟥
bloat 2267.93 ±21.56 2267.93 ±21.56 2275.0 2290.12 ±24.21 2290.12 ±24.21 2290.0 +0.98% +0.98%
fop 590.98 ±1.28 590.98 ±1.28 591.0 592.25 ±1.43 591.85 ±1.20 ⚠️ 1 removed 591.5 +0.22% +0.15%
hsqldb 634.00 ±12.91 634.00 ±12.91 645.5 644.83 ±7.87 644.83 ±7.87 646.5 +1.71% +1.71% 🟥
jython 1681.55 ±7.66 1681.55 ±7.66 1684.0 1696.10 ±7.49 1696.10 ±7.49 1696.5 +0.87% +0.87%
luindex 2301.43 ±6.74 2299.31 ±5.35 ⚠️ 1 removed 2303.0 2297.35 ±8.46 2297.35 ±8.46 2304.0 -0.18% -0.09%
lusearch 574.75 ±13.93 574.75 ±13.93 576.0 646.92 ±10.99 646.92 ±10.99 647.0 +12.56% +12.56% 🟥
pmd 1250.12 ±6.50 1250.12 ±6.50 1249.0 1255.15 ±6.12 1255.15 ±6.12 1256.0 +0.40% +0.40%
xalan 509.35 ±3.45 508.31 ±2.80 ⚠️ 1 removed 507.5 518.00 ±4.18 518.00 ±4.18 519.0 +1.70% +1.91% 🟥

@github-actions
Copy link

OpenJDK

SemiSpace (wrench-2021-05-21-Fri-005347)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 414.57 ±13.74 414.57 ±13.74 390.0 410.05 ±11.23 ⚠️ 3/40 failed 410.05 ±11.23 389.0 -1.09% -1.09% 🟩
eclipse 7415.82 ±15.14 7415.82 ±15.14 7387.0 7445.45 ±45.17 7426.77 ±25.42 ⚠️ 1 removed 7413.5 +0.40% +0.15%
fop 475.07 ±0.98 474.77 ±0.77 ⚠️ 1 removed 474.5 475.25 ±1.53 474.39 ±0.99 ⚠️ 2 removed 475.0 +0.04% -0.08%
hsqldb 433.65 ±2.65 432.08 ±1.53 ⚠️ 2 removed 432.5 433.77 ±1.49 433.77 ±1.49 434.0 +0.03% +0.39%
pmd 1220.28 ±3.39 1220.28 ±3.39 1219.0 1221.80 ±3.51 1221.80 ±3.51 1221.5 +0.12% +0.12%

GenCopy (wrench-2021-05-21-Fri-030026)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 402.90 ±1.88 402.90 ±1.88 403.0 403.10 ±0.71 402.90 ±0.59 ⚠️ 1 removed 403.0 +0.05% -0.00%
eclipse 7938.98 ±26.97 7930.03 ±20.53 ⚠️ 1 removed 7919.5 7929.60 ±18.29 7923.31 ±13.48 ⚠️ 1 removed 7922.5 -0.12% -0.08%
fop 482.90 ±0.90 482.90 ±0.90 483.0 483.18 ±0.96 482.87 ±0.76 ⚠️ 1 removed 483.0 +0.06% -0.01%
hsqldb 370.85 ±3.40 370.85 ±3.40 373.0 368.40 ±3.27 368.40 ±3.27 369.0 -0.66% -0.66%
pmd 1455.22 ±17.89 1450.82 ±15.93 ⚠️ 1 removed 1438.5 1461.65 ±16.29 1461.65 ±16.29 1444.0 +0.44% +0.75%

@github-actions
Copy link

OpenJDK Micro Benchmarks

Running: ['rebench', 'microbm.conf', 'CI_SemiSpace']

Benchmark Trunk (ms) Branch (ms) Diff
BinaryTrees 1910.05 1917.36 +0.38%
Fasta 934.58 938.29 +0.40%
GCBenchMT 1434.98 1435.36 +0.03%
GCBenchST 107.67 107.22 -0.42%
ReverseComplement 52.19 52.54 +0.67%

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

LGTM

src/scheduler/work_counter.rs Outdated Show resolved Hide resolved
src/scheduler/work_counter.rs Show resolved Hide resolved
src/scheduler/stat.rs Show resolved Hide resolved
@qinsoon
Copy link
Member

qinsoon commented May 20, 2021

CI reported that lusearch on JikesRVM is slower by 12%. Is that just noise? Can you check it? Maybe run that benchmark alone to verify. @caizixian

Co-authored-by: Yi Lin <qinsoon@gmail.com>
@caizixian caizixian removed the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label May 21, 2021
@github-actions
Copy link

JikesRVM

NoGC (wrench-2021-05-21-Fri-135349)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 883.65 ±105.89 831.54 ±10.39 ⚠️ 1 removed 833.5 947.42 ±224.60 836.49 ±9.99 ⚠️ 1 removed 833.5 +7.22% +0.60%
fop 627.20 ±1.59 627.20 ±1.59 626.0 627.90 ±1.42 627.90 ±1.42 628.0 +0.11% +0.11%
luindex 2261.45 ±7.48 2259.28 ±6.22 ⚠️ 1 removed 2260.5 2272.68 ±6.33 2272.68 ±6.33 2273.5 +0.50% +0.59%

SemiSpace (wrench-2021-05-21-Fri-140605)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 612.70 ±4.62 612.70 ±4.62 610.5 615.42 ±3.36 615.42 ±3.36 614.0 +0.44% +0.44%
bloat 2288.20 ±24.06 2288.20 ±24.06 2300.5 2307.95 ±33.92 2298.44 ±28.68 ⚠️ 1 removed 2310.5 +0.86% +0.45%
fop 591.27 ±1.28 590.90 ±1.05 ⚠️ 1 removed 591.0 592.38 ±1.32 592.38 ±1.32 591.0 +0.19% +0.25%
hsqldb 646.73 ±10.15 646.73 ±10.15 651.5 642.30 ±14.13 638.15 ±11.67 ⚠️ 1 removed 649.0 -0.68% -1.33% 🟩
jython 1680.58 ±6.87 1680.58 ±6.87 1683.5 1689.45 ±7.22 1689.45 ±7.22 1691.0 +0.53% +0.53%
luindex 2309.28 ±10.97 2305.87 ±8.77 ⚠️ 1 removed 2309.5 2315.22 ±12.88 2310.23 ±8.20 ⚠️ 1 removed 2311.5 +0.26% +0.19%
lusearch 579.23 ±11.06 579.23 ±11.06 584.5 658.05 ±13.77 658.05 ±13.77 654.5 +13.61% +13.61% 🟥
pmd 1262.65 ±6.72 1262.65 ±6.72 1267.0 1251.62 ±8.58 1251.62 ±8.58 1246.5 -0.87% -0.87%
xalan 515.40 ±3.40 515.40 ±3.40 516.0 516.48 ±3.55 516.48 ±3.55 517.5 +0.21% +0.21%

@github-actions
Copy link

OpenJDK

SemiSpace (wrench-2021-05-21-Fri-153521)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 423.19 ±17.34 ⚠️ 4/40 failed 423.19 ±17.34 392.0 413.24 ±12.69 ⚠️ 2/40 failed 413.24 ±12.69 389.0 -2.35% -2.35% 🟩
eclipse 7419.77 ±18.82 7419.77 ±18.82 7394.5 7455.98 ±52.19 7420.61 ±15.54 ⚠️ 2 removed 7417.5 +0.49% +0.01%
fop 476.43 ±0.75 476.43 ±0.75 476.0 474.52 ±0.78 474.52 ±0.78 475.0 -0.40% -0.40%
hsqldb 433.62 ±1.89 432.90 ±1.21 ⚠️ 1 removed 433.0 432.73 ±1.85 432.13 ±1.44 ⚠️ 1 removed 432.0 -0.21% -0.18%
pmd 1224.50 ±3.17 1224.50 ±3.17 1225.0 1225.05 ±3.78 1223.90 ±3.05 ⚠️ 1 removed 1223.0 +0.04% -0.05%

GenCopy (wrench-2021-05-21-Fri-174241)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 403.27 ±0.73 403.27 ±0.73 403.0 403.60 ±0.55 403.60 ±0.55 404.0 +0.08% +0.08%
eclipse 7933.52 ±30.27 7920.97 ±16.93 ⚠️ 1 removed 7919.5 7932.27 ±13.47 7932.27 ±13.47 7926.0 -0.02% +0.14%
fop 482.73 ±0.97 482.49 ±0.87 ⚠️ 1 removed 482.5 482.15 ±0.81 481.92 ±0.68 ⚠️ 1 removed 482.0 -0.12% -0.12%
hsqldb 375.55 ±3.58 374.51 ±2.98 ⚠️ 1 removed 376.0 375.82 ±2.63 375.82 ±2.63 377.0 +0.07% +0.35%
pmd 1461.30 ±18.94 1461.30 ±18.94 1439.5 1445.58 ±16.46 1445.58 ±16.46 1437.5 -1.08% -1.08% 🟩

@github-actions
Copy link

JikesRVM

NoGC (wrench-2021-05-26-Wed-135804)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 774.50 ±5.45 772.33 ±3.33 ⚠️ 1 removed 771.5 778.23 ±2.57 778.23 ±2.57 779.0 +0.48% +0.76%
fop 629.52 ±1.58 629.52 ±1.58 629.0 628.70 ±1.41 628.70 ±1.41 628.0 -0.13% -0.13%
luindex 2263.82 ±7.39 2263.82 ±7.39 2266.5 2269.28 ±6.11 2269.28 ±6.11 2269.5 +0.24% +0.24%

SemiSpace (wrench-2021-05-26-Wed-141002)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 659.50 ±12.55 659.50 ±12.55 660.0 653.67 ±13.08 653.67 ±13.08 658.5 -0.88% -0.88%
bloat 2259.25 ±20.81 2259.25 ±20.81 2264.0 2275.95 ±23.78 2275.95 ±23.78 2281.5 +0.74% +0.74%
fop 590.40 ±1.12 590.40 ±1.12 591.0 591.98 ±1.24 591.98 ±1.24 592.0 +0.27% +0.27%
hsqldb 638.23 ±11.96 638.23 ±11.96 648.5 636.45 ±12.51 636.45 ±12.51 647.0 -0.28% -0.28%
jython 1694.12 ±6.93 1694.12 ±6.93 1695.5 1691.17 ±8.32 1691.17 ±8.32 1694.5 -0.17% -0.17%
luindex 2295.55 ±7.73 2295.55 ±7.73 2299.5 2311.53 ±17.25 2304.54 ±10.16 ⚠️ 1 removed 2306.5 +0.70% +0.39%
lusearch 599.77 ±13.28 599.77 ±13.28 596.0 599.08 ±15.95 599.08 ±15.95 603.5 -0.12% -0.12%
pmd 1254.12 ±7.17 1254.12 ±7.17 1255.5 1259.03 ±7.16 1259.03 ±7.16 1255.0 +0.39% +0.39%
xalan 515.60 ±3.30 515.60 ±3.30 518.5 512.15 ±4.60 510.87 ±3.91 ⚠️ 1 removed 510.5 -0.67% -0.92%

@github-actions
Copy link

OpenJDK

SemiSpace (wrench-2021-05-26-Wed-153957)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 389.61 ±1.30 ⚠️ 2/40 failed 389.61 ±1.30 390.0 387.82 ±0.88 ⚠️ 2/40 failed 387.59 ±0.78 ⚠️ 1 removed 387.0 -0.46% -0.52%
eclipse 7450.62 ±46.73 7420.63 ±20.43 ⚠️ 2 removed 7402.5 7461.68 ±50.65 7438.74 ±20.90 ⚠️ 1 removed 7434.0 +0.15% +0.24%
fop 476.50 ±0.85 476.50 ±0.85 476.5 478.82 ±0.78 478.82 ±0.78 479.0 +0.49% +0.49%
hsqldb 434.88 ±2.56 433.82 ±1.46 ⚠️ 1 removed 433.5 433.05 ±1.20 433.05 ±1.20 433.0 -0.42% -0.18%
pmd 1222.12 ±4.42 1220.87 ±3.72 ⚠️ 1 removed 1221.5 1221.88 ±3.60 1221.88 ±3.60 1218.0 -0.02% +0.08%

GenCopy (wrench-2021-05-26-Wed-174838)

Benchmark Trunk(ms) Branch(ms) Diff
mean mean without outliers median mean mean without outliers median mean mean without outliers
antlr 402.88 ±0.64 402.67 ±0.50 ⚠️ 1 removed 403.0 403.82 ±0.61 403.82 ±0.61 404.0 +0.24% +0.29%
eclipse 8099.90 ±403.66 7900.54 ±18.86 ⚠️ 1 removed 7912.5 7892.05 ±17.49 ⚠️ 1/40 failed 7887.58 ±15.38 ⚠️ 1 removed 7882.0 -2.57% -0.16%
fop 482.82 ±0.75 482.82 ±0.75 483.0 482.45 ±0.65 482.45 ±0.65 482.0 -0.08% -0.08%
hsqldb 369.35 ±3.00 369.35 ±3.00 370.0 373.43 ±2.77 373.43 ±2.77 372.0 +1.10% +1.10% 🟥
pmd 1450.10 ±17.33 1450.10 ±17.33 1446.5 1474.35 ±18.64 1474.35 ±18.64 1465.5 +1.67% +1.67% 🟥

@qinsoon qinsoon added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label May 27, 2021
@caizixian caizixian removed the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label May 27, 2021
@caizixian caizixian merged commit e076435 into master May 28, 2021
@caizixian caizixian deleted the worker-stat branch May 28, 2021 05:41
k-sareen pushed a commit to k-sareen/mmtk-core that referenced this pull request Jun 10, 2021
This PR refactors work-packet level statistics collection.
Work packet execution time measuring was hardcoded. It's now abstracted through a WorkCounter trait interface.
This PR also makes mmtk#315 easier.
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