Skip to content

Commit 986cfcd

Browse files
authored
Enable inefficient_to_string Clippy lint (apache#17583)
Enable `inefficient_to_string` Clippy lint and update the code. The code changes were done by Clippy `--fix`. The lint addresses a Rust's type system or stdlib limitation, which results in `(&&String).to_string()` being a lot slower than `(&String).to_string()` and `(*(&&String)).to_string()`. The difference is 40% in my measurements on 10-character long strings. The difference comes from the fact that the faster code paths are optimized for `String` and the `&&` double reference somehow prevents the compiler from taking such specialized code path.
1 parent 0e9b4a3 commit 986cfcd

File tree

20 files changed

+41
-35
lines changed

20 files changed

+41
-35
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ used_underscore_binding = "warn"
223223
or_fun_call = "warn"
224224
unnecessary_lazy_evaluations = "warn"
225225
uninlined_format_args = "warn"
226+
inefficient_to_string = "warn"
226227

227228
[workspace.lints.rust]
228229
unexpected_cfgs = { level = "warn", check-cfg = [

datafusion-examples/examples/parquet_embedded_index.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ fn write_file_with_index(path: &Path, values: &[&str]) -> Result<()> {
321321

322322
// compute the distinct index
323323
let distinct_index: DistinctIndex =
324-
DistinctIndex::new(values.iter().map(|s| s.to_string()));
324+
DistinctIndex::new(values.iter().map(|s| (*s).to_string()));
325325

326326
let file = File::create(path)?;
327327

datafusion/catalog/src/information_schema.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ impl SchemaProvider for InformationSchemaProvider {
492492
fn table_names(&self) -> Vec<String> {
493493
INFORMATION_SCHEMA_TABLES
494494
.iter()
495-
.map(|t| t.to_string())
495+
.map(|t| (*t).to_string())
496496
.collect()
497497
}
498498

datafusion/common/src/error.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -455,12 +455,13 @@ impl DataFusionError {
455455
/// If backtrace enabled then error has a format "message" [`Self::BACK_TRACE_SEP`] "backtrace"
456456
/// The method strips the backtrace and outputs "message"
457457
pub fn strip_backtrace(&self) -> String {
458-
self.to_string()
458+
(*self
459+
.to_string()
459460
.split(Self::BACK_TRACE_SEP)
460461
.collect::<Vec<&str>>()
461462
.first()
462-
.unwrap_or(&"")
463-
.to_string()
463+
.unwrap_or(&""))
464+
.to_string()
464465
}
465466

466467
/// To enable optional rust backtrace in DataFusion:

datafusion/core/src/dataframe/parquet.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ mod tests {
205205
&HashMap::from_iter(
206206
[("datafusion.execution.batch_size", "10")]
207207
.iter()
208-
.map(|(s1, s2)| (s1.to_string(), s2.to_string())),
208+
.map(|(s1, s2)| ((*s1).to_string(), (*s2).to_string())),
209209
),
210210
)?);
211211
register_aggregate_csv(&ctx, "aggregate_test_100").await?;

datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ async fn run_aggregate_test(input1: Vec<RecordBatch>, group_by_columns: Vec<&str
337337
];
338338
let expr = group_by_columns
339339
.iter()
340-
.map(|elem| (col(elem, &schema).unwrap(), elem.to_string()))
340+
.map(|elem| (col(elem, &schema).unwrap(), (*elem).to_string()))
341341
.collect::<Vec<_>>();
342342
let group_by = PhysicalGroupBy::new_single(expr);
343343

datafusion/core/tests/fuzz_cases/aggregation_fuzzer/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ pub(crate) fn check_equality_of_batches(
7777
if lhs_row != rhs_row {
7878
return Err(InconsistentResult {
7979
row_idx,
80-
lhs_row: lhs_row.to_string(),
81-
rhs_row: rhs_row.to_string(),
80+
lhs_row: (*lhs_row).to_string(),
81+
rhs_row: (*rhs_row).to_string(),
8282
});
8383
}
8484
}

datafusion/core/tests/fuzz_cases/equivalence/projection.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ fn project_orderings_random() -> Result<()> {
7373
for proj_exprs in proj_exprs.iter().combinations(n_req) {
7474
let proj_exprs = proj_exprs
7575
.into_iter()
76-
.map(|(expr, name)| (Arc::clone(expr), name.to_string()))
76+
.map(|(expr, name)| (Arc::clone(expr), (*name).to_string()))
7777
.collect::<Vec<_>>();
7878
let (projected_batch, projected_eq) = apply_projection(
7979
proj_exprs.clone(),
@@ -147,7 +147,7 @@ fn ordering_satisfy_after_projection_random() -> Result<()> {
147147
for proj_exprs in proj_exprs.iter().combinations(n_req) {
148148
let proj_exprs = proj_exprs
149149
.into_iter()
150-
.map(|(expr, name)| (Arc::clone(expr), name.to_string()));
150+
.map(|(expr, name)| (Arc::clone(expr), (*name).to_string()));
151151
let (projected_batch, projected_eq) = apply_projection(
152152
proj_exprs.clone(),
153153
&table_data_with_properties,

datafusion/core/tests/fuzz_cases/sort_fuzz.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ impl SortTest {
188188
}
189189

190190
fn with_sort_columns(mut self, sort_columns: Vec<&str>) -> Self {
191-
self.sort_columns = sort_columns.iter().map(|s| s.to_string()).collect();
191+
self.sort_columns = sort_columns.iter().map(|s| (*s).to_string()).collect();
192192
self
193193
}
194194

datafusion/core/tests/fuzz_cases/topk_filter_pushdown.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ async fn test_fuzz_topk_filter_pushdown() {
297297
order_vec.push(ordering);
298298
}
299299
None => {
300-
orders.insert(order_column.to_string(), vec![ordering]);
300+
orders.insert((*order_column).to_string(), vec![ordering]);
301301
}
302302
}
303303
}

0 commit comments

Comments
 (0)