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

Zero new memory after resizing ColumnChunkData #5082

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

benjaminwinger
Copy link
Collaborator

From #5050; uninitialized memory ends up being used for statistics in the relationship data. I suspect there may also be an issue in RelBatchInsert::appendNodeGroup, since if I understand that correctly it should be filling in both the data and the gaps; so this shouldn't matter. But regardless I think the behaviour of resize should match the default behaviour of the constructor of initializing the data to zero.

Copy link

Benchmark Result

Master commit hash: a573d705b47cf22865fecb9b4142223a84f7efcf
Branch commit hash: f41b1de4a0b75c553f9a13fc79c283e9e5465b62

Query Group Query Name Mean Time - Commit (ms) Mean Time - Master (ms) Diff
aggregation q24 738.96 727.24 11.72 (1.61%)
aggregation q28 6596.49 6576.19 20.30 (0.31%)
filter q14 128.85 124.79 4.06 (3.26%)
filter q15 128.16 131.47 -3.32 (-2.52%)
filter q16 350.86 341.24 9.62 (2.82%)
filter q17 452.99 453.60 -0.61 (-0.13%)
filter q18 1956.96 1914.43 42.54 (2.22%)
filter zonemap-node 90.88 88.83 2.05 (2.31%)
filter zonemap-node-lhs-cast 90.45 89.03 1.42 (1.59%)
filter zonemap-node-null 90.02 88.70 1.32 (1.49%)
filter zonemap-rel 5728.21 5770.72 -42.51 (-0.74%)
fixed_size_expr_evaluator q07 680.97 682.51 -1.54 (-0.23%)
fixed_size_expr_evaluator q08 969.18 980.92 -11.74 (-1.20%)
fixed_size_expr_evaluator q09 978.91 980.88 -1.97 (-0.20%)
fixed_size_expr_evaluator q10 259.80 257.25 2.55 (0.99%)
fixed_size_expr_evaluator q11 259.30 257.85 1.45 (0.56%)
fixed_size_expr_evaluator q12 233.54 233.73 -0.19 (-0.08%)
fixed_size_expr_evaluator q13 1576.04 1589.88 -13.84 (-0.87%)
fixed_size_seq_scan q23 113.67 113.80 -0.14 (-0.12%)
join q29 763.00 711.83 51.17 (7.19%)
join q30 1656.86 1529.89 126.96 (8.30%)
join q31 6.53 4.99 1.54 (30.91%)
join SelectiveTwoHopJoin 48.25 45.17 3.08 (6.82%)
ldbc_snb_ic q35 10.53 10.59 -0.06 (-0.55%)
ldbc_snb_ic q36 92.22 96.12 -3.91 (-4.06%)
ldbc_snb_is q32 2.11 2.51 -0.40 (-15.87%)
ldbc_snb_is q33 12.31 11.14 1.17 (10.50%)
ldbc_snb_is q34 1.25 1.30 -0.05 (-3.89%)
multi-rel multi-rel-large-scan 1936.11 1894.95 41.16 (2.17%)
multi-rel multi-rel-lookup 5.94 6.78 -0.83 (-12.29%)
multi-rel multi-rel-small-scan 207.21 206.75 0.45 (0.22%)
order_by q25 135.00 130.84 4.16 (3.18%)
order_by q26 450.86 467.16 -16.30 (-3.49%)
order_by q27 1394.43 1398.56 -4.14 (-0.30%)
recursive_join recursive-join-bidirection 290.22 303.05 -12.83 (-4.23%)
recursive_join recursive-join-dense 7193.73 7152.78 40.95 (0.57%)
recursive_join recursive-join-path 23355.22 23414.12 -58.91 (-0.25%)
recursive_join recursive-join-sparse 637.94 636.05 1.89 (0.30%)
recursive_join recursive-join-trail 7144.86 7086.12 58.75 (0.83%)
scan_after_filter q01 172.82 169.73 3.09 (1.82%)
scan_after_filter q02 157.97 156.02 1.96 (1.25%)
shortest_path_ldbc100 q37 83.26 87.93 -4.67 (-5.31%)
shortest_path_ldbc100 q38 338.87 309.58 29.29 (9.46%)
shortest_path_ldbc100 q39 64.20 58.43 5.77 (9.87%)
shortest_path_ldbc100 q40 374.63 392.21 -17.59 (-4.48%)
var_size_expr_evaluator q03 2088.91 2137.41 -48.50 (-2.27%)
var_size_expr_evaluator q04 2262.91 2176.86 86.04 (3.95%)
var_size_expr_evaluator q05 2754.11 2740.77 13.34 (0.49%)
var_size_expr_evaluator q06 1363.37 1392.12 -28.75 (-2.06%)
var_size_seq_scan q19 1452.66 1433.80 18.86 (1.32%)
var_size_seq_scan q20 2685.05 2668.68 16.38 (0.61%)
var_size_seq_scan q21 2277.26 2300.57 -23.30 (-1.01%)
var_size_seq_scan q22 128.15 125.79 2.36 (1.87%)

Copy link

codecov bot commented Mar 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.52%. Comparing base (fb62bab) to head (9190c5b).
Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5082      +/-   ##
==========================================
- Coverage   87.99%   87.52%   -0.47%     
==========================================
  Files        1418     1403      -15     
  Lines       64597    63557    -1040     
  Branches     7627     7509     -118     
==========================================
- Hits        56840    55631    -1209     
- Misses       7728     7897     +169     
  Partials       29       29              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@andyfengHKU andyfengHKU merged commit 9e4e068 into master Mar 21, 2025
28 checks passed
@andyfengHKU andyfengHKU deleted the fix-uninit-chunk-mem branch March 21, 2025 06:31
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.

None yet

3 participants