Rich-T-kid commented on code in PR #11904:
URL: https://github.com/apache/arrow-rs/pull/11904#discussion_r4393788975


##########
arrow-row/src/lib.rs:
##########
@@ -775,9 +777,23 @@ impl Codec {
         match self {
             Codec::Stateless => Ok(Encoder::Stateless),
             Codec::Dictionary(converter, nulls) => {
-                let values = array.as_any_dictionary().values().clone();
+                let dictionary = array.as_any_dictionary();
+                let values = dictionary.values().clone();
+                // Encoding a dictionary costs time proportional to its values,
+                // so when they outnumber the rows, gather only the referenced
+                // values and remap each row's key onto them.
+                let (values, remap) = match values.len() > dictionary.len() {

Review Comment:
   There’s some fixed cost to re-hashing the values to create new mappings. Can 
we benchmark to find out what a decent cutoff would be?
   
   Record batches are typically 8192 rows, so we wouldn’t want to incur this 
cost when there are only 10k unique values. Or maybe we would 🤔. I think we 
should have some numbers to back up this decision.



-- 
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]

Reply via email to