CuteChuanChuan commented on PR #25652:
URL: https://github.com/apache/datafusion/pull/25652#issuecomment-5913016708
Hi @jayzhan211,
About commit 40bfa2b, I went through three steps:
1. Applied your two suggestions (keep the guard/fallback, read the cache
inline).
`repeated_intern_emit` with 65536 rows / card 65536 still regressed,
+25.7%
and +44.9% over two runs. In that bench every batch has a new values
array:
`vectorized_append` syncs the cache first, so `cached` is true, but
`val_to_inner` only has the values that were just appended. Most rows in
`vectorized_equal_to` then miss and go through `value_dedup`.
2. Changed the miss path (`lookup_inner_slot` -> `equal_to_uncached`). Since
we
already have `lhs_slot`, it compares the value directly to
`inner[lhs_slot]`.
If they are equal, that mapping is correct (each distinct value has one
slot
in `inner`), so it goes into `val_to_inner`. If not, nothing is cached.
That
fixed the case above, but `intern_emit` with low cardinality was still
+5% to +11%.
3. Borrowed `val_to_inner` / `group_to_inner` as slices before the loop, so
the
loop no longer calls a `&mut self` method on a miss. With that, everything
is back to main or faster; the table is in the PR description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
