Prefer to_string to format!("{}") (#7581)

As per benchmarks in #7568, using
format! as opposed to calling to_string directly has ~100ns
overhead. There’s no reason not to get rid of that overhead especially
since using to_string is actually shorter to type.

Note that it may be beneficial to further optimise integer formatting
by using fixed-size buffer and custom conversion which doesn’t use
std.  This optimisation is outside the scope of this commit.

chain/chain/src/types.rs

@@ -629,9 +629,8 @@ pub trait RuntimeAdapter: Send + Sync {
             "apply_transactions",
             shard_id)
         .entered();
-        let _timer = metrics::APPLYING_CHUNKS_TIME
-            .with_label_values(&[&format!("{}", shard_id)])
-            .start_timer();
+        let _timer =
+            metrics::APPLYING_CHUNKS_TIME.with_label_values(&[&shard_id.to_string()]).start_timer();
         self.apply_transactions_with_optional_storage_proof(
             shard_id,
             state_root,

chain/chunks/src/lib.rs

@@ -2180,7 +2180,7 @@ impl ShardsManager {
         shard_id: ShardId,
     ) -> Result<(), Error> {
         let _timer = metrics::DISTRIBUTE_ENCODED_CHUNK_TIME
-            .with_label_values(&[&format!("{}", shard_id)])
+            .with_label_values(&[&shard_id.to_string()])
             .start_timer();
         // TODO: if the number of validators exceeds the number of parts, this logic must be changed
         let chunk_header = encoded_chunk.cloned_header();

chain/client/src/client.rs

@@ -620,9 +620,8 @@ impl Client {
         shard_id: ShardId,
     ) -> Result<Option<(EncodedShardChunk, Vec<MerklePath>, Vec<Receipt>)>, Error> {
         let timer = Instant::now();
-        let _timer = metrics::PRODUCE_CHUNK_TIME
-            .with_label_values(&[&format!("{}", shard_id)])
-            .start_timer();
+        let _timer =
+            metrics::PRODUCE_CHUNK_TIME.with_label_values(&[&shard_id.to_string()]).start_timer();
         let _span = tracing::debug_span!(target: "client", "produce_chunk", next_height, shard_id, ?epoch_id).entered();
         let validator_signer = self
             .validator_signer
@@ -1340,7 +1339,7 @@ impl Client {
                             ?shard_id)
                         .entered();
                         let _timer = metrics::PRODUCE_AND_DISTRIBUTE_CHUNK_TIME
-                            .with_label_values(&[&format!("{}", shard_id)])
+                            .with_label_values(&[&shard_id.to_string()])
                             .start_timer();
                         match self.produce_chunk(
                             *block.hash(),

chain/client/src/info.rs

@@ -84,13 +84,13 @@ impl InfoHelper {
 
     pub fn chunk_processed(&mut self, shard_id: ShardId, gas_used: Gas, balance_burnt: Balance) {
         metrics::TGAS_USAGE_HIST
-            .with_label_values(&[&format!("{}", shard_id)])
+            .with_label_values(&[&shard_id.to_string()])
             .observe(gas_used as f64 / TERAGAS);
         metrics::BALANCE_BURNT.inc_by(balance_burnt as f64);
     }
 
     pub fn chunk_skipped(&mut self, shard_id: ShardId) {
-        metrics::CHUNK_SKIPPED_TOTAL.with_label_values(&[&format!("{}", shard_id)]).inc();
+        metrics::CHUNK_SKIPPED_TOTAL.with_label_values(&[&shard_id.to_string()]).inc();
     }
 
     pub fn block_processed(

chain/client/src/view_client.rs

@@ -738,7 +738,7 @@ impl Handler<GetStateChangesWithCauseInBlockForTrackedShards> for ViewClientActo
             {
                 Ok(shard_id) => shard_id,
                 Err(err) => {
-                    return Err(GetStateChangesError::IOError { error_message: format!("{}", err) })
+                    return Err(GetStateChangesError::IOError { error_message: err.to_string() })
                 }
             };
 

chain/jsonrpc-primitives/src/message.rs

@@ -254,7 +254,7 @@ pub fn decoded_to_parsed(res: JsonResult<WireMessage>) -> Parsed {
         Ok(WireMessage::Message(Message::UnmatchedSub(value))) => Err(Broken::Unmatched(value)),
         Ok(WireMessage::Message(m)) => Ok(m),
         Ok(WireMessage::Broken(b)) => Err(b),
-        Err(e) => Err(Broken::SyntaxError(format!("{}", e))),
+        Err(e) => Err(Broken::SyntaxError(e.to_string())),
     }
 }
 

chain/network/src/network_protocol/tests.rs

@@ -111,7 +111,7 @@ fn serialize_deserialize() -> anyhow::Result<()> {
         for m in &msgs {
             (|| {
                 let m2 = PeerMessage::deserialize(enc, &m.serialize(enc))
-                    .with_context(|| format!("{m}"))?;
+                    .with_context(|| m.to_string())?;
                 if *m != m2 {
                     bail!("deserialize(serialize({m}) = {m2}");
                 }

core/crypto/src/signature.rs

@@ -831,7 +831,7 @@ impl serde::Serialize for Signature {
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(&format!("{}", self))
+        serializer.serialize_str(&self.to_string())
     }
 }
 

core/o11y/benches/metrics.rs

@@ -14,7 +14,7 @@ const NUM_SHARDS: usize = 8;
 fn inc_counter_vec_with_label_values(bench: &mut Bencher) {
     bench.iter(|| {
         for shard_id in 0..NUM_SHARDS {
-            COUNTERS.with_label_values(&[&format!("{}", shard_id)]).inc();
+            COUNTERS.with_label_values(&[&shard_id.to_string()]).inc();
         }
     });
 }
@@ -75,7 +75,7 @@ fn inc_counter_vec_with_label_values_stack_no_format(bench: &mut Bencher) {
 fn inc_counter_vec_cached(bench: &mut Bencher) {
     const NUM_SHARDS: usize = 8;
     let counters: Vec<IntCounter> = (0..NUM_SHARDS)
-        .map(|shard_id| COUNTERS.with_label_values(&[&format!("{}", shard_id)]))
+        .map(|shard_id| COUNTERS.with_label_values(&[&shard_id.to_string()]))
         .collect();
     bench.iter(|| {
         for shard_id in 0..NUM_SHARDS {
@@ -86,7 +86,7 @@ fn inc_counter_vec_cached(bench: &mut Bencher) {
 
 fn inc_counter_vec_cached_str(bench: &mut Bencher) {
     const NUM_SHARDS: usize = 8;
-    let shard_ids: Vec<String> = (0..NUM_SHARDS).map(|shard_id| format!("{}", shard_id)).collect();
+    let shard_ids: Vec<String> = (0..NUM_SHARDS).map(|shard_id| shard_id.to_string()).collect();
     bench.iter(|| {
         for shard_id in 0..NUM_SHARDS {
             COUNTERS.with_label_values(&[&shard_ids[shard_id]]).inc();

core/o11y/src/io_tracer.rs

@@ -302,7 +302,7 @@ impl tracing::field::Visit for SpanInfo {
     fn record_str(&mut self, field: &tracing::field::Field, value: &str) {
         // "count" is a special field, everything else are key values pairs.
         if field.name() == "counter" {
-            *self.counts.entry(format!("{value}")).or_default() += 1;
+            *self.counts.entry(value.to_string()).or_default() += 1;
         } else {
             self.record_debug(field, &value);
         }

core/primitives/src/validator_signer.rs

@@ -180,7 +180,7 @@ impl ValidatorSigner for InMemoryValidatorSigner {
     fn sign_telemetry(&self, info: &TelemetryInfo) -> serde_json::Value {
         let mut value = serde_json::to_value(info).expect("Telemetry must serialize to JSON");
         let content = serde_json::to_string(&value).expect("Telemetry must serialize to JSON");
-        value["signature"] = format!("{}", self.signer.sign(content.as_bytes())).into();
+        value["signature"] = self.signer.sign(content.as_bytes()).to_string().into();
         value
     }
 

core/store/src/db/colddb.rs

@@ -438,7 +438,7 @@ mod test {
         for col in [DBCol::BlockHeader, DBCol::Block, DBCol::EpochInfo, DBCol::ChunkHashesByHeight]
         {
             let dbs: [(bool, &dyn Database); 2] = [(false, &db), (true, &db.0)];
-            result.push(format!("{col}"));
+            result.push(col.to_string());
             for (is_raw, db) in dbs {
                 let name = if is_raw { "raw " } else { "cold" };
                 for item in db.iter(col) {

core/store/src/trie/shard_tries.rs

@@ -216,7 +216,7 @@ impl ShardTries {
         store_update: &mut StoreUpdate,
     ) {
         metrics::APPLIED_TRIE_INSERTIONS
-            .with_label_values(&[&format!("{}", shard_uid.shard_id)])
+            .with_label_values(&[&shard_uid.shard_id.to_string()])
             .inc_by(trie_changes.insertions.len() as u64);
         self.apply_insertions_inner(&trie_changes.insertions, shard_uid, store_update)
     }
@@ -228,7 +228,7 @@ impl ShardTries {
         store_update: &mut StoreUpdate,
     ) {
         metrics::APPLIED_TRIE_DELETIONS
-            .with_label_values(&[&format!("{}", shard_uid.shard_id)])
+            .with_label_values(&[&shard_uid.shard_id.to_string()])
             .inc_by(trie_changes.deletions.len() as u64);
         self.apply_deletions_inner(&trie_changes.deletions, shard_uid, store_update)
     }
@@ -240,7 +240,7 @@ impl ShardTries {
         store_update: &mut StoreUpdate,
     ) {
         metrics::REVERTED_TRIE_INSERTIONS
-            .with_label_values(&[&format!("{}", shard_uid.shard_id)])
+            .with_label_values(&[&shard_uid.shard_id.to_string()])
             .inc_by(trie_changes.insertions.len() as u64);
         self.apply_deletions_inner(&trie_changes.insertions, shard_uid, store_update)
     }

core/store/src/trie/trie_storage.rs

@@ -97,7 +97,7 @@ impl TrieCacheInner {
         is_view: bool,
     ) -> Self {
         assert!(cache_capacity > 0 && total_size_limit > 0);
-        let metrics_labels: [&str; 2] = [&format!("{}", shard_id), &format!("{}", is_view as u8)];
+        let metrics_labels: [&str; 2] = [&shard_id.to_string(), if is_view { "1" } else { "0" }];
         let metrics = TrieCacheMetrics {
             shard_cache_too_large: metrics::SHARD_CACHE_TOO_LARGE
                 .with_label_values(&metrics_labels),
@@ -382,7 +382,7 @@ impl TrieCachingStorage {
         is_view: bool,
     ) -> TrieCachingStorage {
         let metrics_labels: [&str; 2] =
-            [&format!("{}", shard_uid.shard_id), &format!("{}", is_view as u8)];
+            [&shard_uid.shard_id.to_string(), if is_view { "1" } else { "0" }];
         let metrics = TrieCacheInnerMetrics {
             chunk_cache_hits: metrics::CHUNK_CACHE_HITS.with_label_values(&metrics_labels),
             chunk_cache_misses: metrics::CHUNK_CACHE_MISSES.with_label_values(&metrics_labels),

docs/style.md

@@ -153,6 +153,23 @@ mind that compilers are pretty good at optimising and in practice may generate
 optimal code anyway.  Furthermore, optimising code for readability may be more
 important (especially outside of hot path) than small performance gains.
 
+### Prefer `to_string` to `format!("{}")`
+
+Prefer calling `to_string` method on an object rather than passing it through
+`format!("{}")` if all you’re doing is converting it to a `String`.
+
+```rust
+// GOOD
+lat hash = block_hash.to_string();
+let msg = format!("{}: failed to open", path.display());
+
+// BAD
+lat hash = format!("{block_hash}");
+let msg = path.display() + ": failed to open";
+```
+
+**Rationale:** `to_string` is shorter to type and also faster.
+
 ### Import Granularity
 
 Group import by module, but not deeper:

genesis-tools/keypair-generator/src/main.rs

@@ -81,7 +81,7 @@ fn main() {
 
                 pks.push(key.public_key());
             }
-            let pks: Vec<_> = pks.into_iter().map(|pk| format!("{}", pk)).collect();
+            let pks: Vec<_> = pks.into_iter().map(|pk| pk.to_string()).collect();
             println!("List of public keys:");
             println!("{}", pks.join(","));
         }

integration-tests/src/tests/client/sharding_upgrade.rs

@@ -567,15 +567,15 @@ fn gen_cross_contract_transaction(
                     "id": 0 },
                 {"action_transfer": {
                     "promise_index": 0,
-                    "amount": format!("{}", NEAR_BASE),
+                    "amount": NEAR_BASE.to_string(),
                 }, "id": 0 },
                 {"action_add_key_with_full_access": {
                     "promise_index": 0,
                     "public_key": to_base64(&signer_new_account.public_key.try_to_vec().unwrap()),
                     "nonce": 0,
                 }, "id": 0 }
             ],
-        "amount": format!("{}", NEAR_BASE),
+        "amount": NEAR_BASE.to_string(),
         "gas": GAS_2,
         }, "id": 1}
     ]);

runtime/runtime/tests/test_async_calls.rs

@@ -683,13 +683,13 @@ fn test_account_factory() {
         }, "id": 0 },
         {"action_transfer": {
             "promise_index": 0,
-            "amount": format!("{}", TESTING_INIT_BALANCE / 2),
+            "amount": (TESTING_INIT_BALANCE / 2).to_string(),
         }, "id": 0 },
         {"action_add_key_with_function_call": {
             "promise_index": 0,
             "public_key": to_base64(&signer_new_account.public_key.try_to_vec().unwrap()),
             "nonce": 0,
-            "allowance": format!("{}", TESTING_INIT_BALANCE / 2),
+            "allowance": (TESTING_INIT_BALANCE / 2).to_string(),
             "receiver_id": "near_1",
             "method_names": "call_promise,hello"
         }, "id": 0 },
@@ -840,7 +840,7 @@ fn test_create_account_add_key_call_delete_key_delete_account() {
         }, "id": 0 },
         {"action_transfer": {
             "promise_index": 0,
-            "amount": format!("{}", TESTING_INIT_BALANCE / 2),
+            "amount": (TESTING_INIT_BALANCE / 2).to_string(),
         }, "id": 0 },
         {"action_add_key_with_full_access": {
             "promise_index": 0,
@@ -967,7 +967,7 @@ fn test_transfer_64len_hex() {
         }, "id": 0 },
         {"action_transfer": {
             "promise_index": 0,
-            "amount": format!("{}", TESTING_INIT_BALANCE / 2),
+            "amount": (TESTING_INIT_BALANCE / 2).to_string(),
         }, "id": 0 },
     ]);
 
@@ -1033,7 +1033,7 @@ fn test_create_transfer_64len_hex_fail() {
         }, "id": 0 },
         {"action_transfer": {
             "promise_index": 0,
-            "amount": format!("{}", TESTING_INIT_BALANCE / 2),
+            "amount": (TESTING_INIT_BALANCE / 2).to_string(),
         }, "id": 0 },
     ]);
 

test-utils/runtime-tester/fuzz/fuzz_targets/runtime_fuzzer.rs

@@ -5,7 +5,7 @@ use std::fs::File;
 use std::time::Duration;
 
 fn do_fuzz(scenario: &Scenario) -> Result<(), String> {
-    let stats = scenario.run().result.map_err(|e| format!("{}", e))?;
+    let stats = scenario.run().result.map_err(|e| e.to_string())?;
     for block_stats in stats.blocks_stats {
         if block_stats.block_production_time > Duration::from_secs(2) {
             return Err(format!(