perf: cache dictionary arc pointer

[Rich-T-kid]

Which issue does this PR close?

• Closes cache dictionary arc ptr in groupColumn #23921.
• this PR was created on top of introduce new dictionary benchmarks #24416, so once that is merged in we can rebase this branch

Rationale for this change

append_val is called once per new group on the scalar (streaming) code path. Previously it hashed a single-element slice of the dictionary values array on every call, paying create_hashes fixed overhead 65k times for a high-cardinality batch. Caching the full values array hash keyed on Arc::ptr_eq collapses that to one vectorized hash pass per batch, yielding a 2× speedup on the all-unique case with no measurable regression elsewhere.

What changes are included in this PR?

• DictionaryGroupValuesColumn gains a cached_values: Option field
• append_val: on a cache miss (Arc::ptr_eq fails), hashes the entire dict.values() array into val_hashes and stores the Arc; on a hit, reuses val_hashes[val_idx] directly — eliminating the per-call slice(val_idx, 1) allocation and create_hashes call

Are these changes tested?

nothing logically changed.

Are there any user-facing changes?

no

[Rich-T-kid]

cc @zhuqi-lucas since this was a follow up you mentioned 😄

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.21%. Comparing base (bbf739a) to head (d22b079).
⚠️ Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
...gregates/group_values/multi_group_by/dictionary.rs	94.44%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24418      +/-   ##
==========================================
- Coverage   81.23%   81.21%   -0.02%     
==========================================
  Files        1111     1113       +2     
  Lines      390208   392265    +2057     
  Branches   390208   392265    +2057     
==========================================
+ Hits       316990   318591    +1601     
- Misses      54591    54924     +333     
- Partials    18627    18750     +123     

☔ View full report in Codecov by Harness.
📢 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.

[Rich-T-kid]

CI failures are not related

[saadtajwar]

Nice!

[alamb]

Thank you @Rich-T-kid -- this looks quite clever and can probably generate an observable speedup for end to end benchmarks that read data as parquet dictionaries. Did you try that?

Also, I wonder if it would be easier to test some of these dictionary related improvements if you added a config setting to treat string columns in parquet files as dictionary (rather than Stringview) 🤔

[Rich-T-kid]

Thank you @Rich-T-kid -- this looks quite clever and can probably generate an observable speedup for end to end benchmarks that read data as parquet dictionaries. Did you try that?

Also, I wonder if it would be easier to test some of these dictionary related improvements if you added a config setting to treat string columns in parquet files as dictionary (rather than Stringview) 🤔

@alamb no not exactly. I have made some made tweaks locally to the existing tpch benchmarks by changing datatypes to dict and they've shown improvement.

I have this PR #24227 that implements your suggestion #24111 (comment) , it would be nice if we could run the tpch benchmarks on it while having the flag enabled. I think we could see some very nice results.

Ill tweak this adjust this PR

[Rich-T-kid]

once #24416 we can run the benchmarks on this PR, locally i saw up to a 50% boost for the high cardinality branch