From db4b2f92590bccaeac99d05c38bd56dd618c33d0 Mon Sep 17 00:00:00 2001
From: Lukas Wirth <lukastw97@gmail.com>
Date: Sun, 15 Sep 2024 12:39:53 +0200
Subject: [PATCH] lib: Optimize `ChangeId::reverse_hex`

---
 cli/src/cli_util.rs         | 15 +++++++++------
 cli/src/commit_templater.rs |  7 +------
 lib/src/backend.rs          | 13 +++++++++++++
 lib/src/hex_util.rs         | 18 ++++++++++++++----
 4 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index c39e8805f5..66ba936f41 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -63,7 +63,6 @@ use jj_lib::git;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::gitignore::GitIgnoreError;
 use jj_lib::gitignore::GitIgnoreFile;
-use jj_lib::hex_util::to_reverse_hex;
 use jj_lib::id_prefix::IdPrefixContext;
 use jj_lib::matchers::Matcher;
 use jj_lib::merge::MergedTreeValue;
@@ -2482,17 +2481,21 @@ pub fn edit_temp_file(
 }
 
 pub fn short_commit_hash(commit_id: &CommitId) -> String {
-    commit_id.hex()[0..12].to_string()
+    let mut hash = commit_id.hex();
+    hash.truncate(12);
+    hash
 }
 
 pub fn short_change_hash(change_id: &ChangeId) -> String {
-    // TODO: We could avoid the unwrap() and make this more efficient by converting
-    // straight from binary.
-    to_reverse_hex(&change_id.hex()[0..12]).unwrap()
+    let mut hash = change_id.reverse_hex();
+    hash.truncate(12);
+    hash
 }
 
 pub fn short_operation_hash(operation_id: &OperationId) -> String {
-    operation_id.hex()[0..12].to_string()
+    let mut hash = operation_id.hex();
+    hash.truncate(12);
+    hash
 }
 
 /// Wrapper around a `DiffEditor` to conditionally start interactive session.
diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs
index acf9d580a6..5ca1a53228 100644
--- a/cli/src/commit_templater.rs
+++ b/cli/src/commit_templater.rs
@@ -30,7 +30,6 @@ use jj_lib::extensions_map::ExtensionsMap;
 use jj_lib::fileset;
 use jj_lib::fileset::FilesetExpression;
 use jj_lib::git;
-use jj_lib::hex_util::to_reverse_hex;
 use jj_lib::id_prefix::IdPrefixContext;
 use jj_lib::matchers::Matcher;
 use jj_lib::merged_tree::MergedTree;
@@ -1180,11 +1179,7 @@ impl CommitOrChangeId {
     pub fn hex(&self) -> String {
         match self {
             CommitOrChangeId::Commit(id) => id.hex(),
-            CommitOrChangeId::Change(id) => {
-                // TODO: We can avoid the unwrap() and make this more efficient by converting
-                // straight from bytes.
-                to_reverse_hex(&id.hex()).unwrap()
-            }
+            CommitOrChangeId::Change(id) => id.reverse_hex(),
         }
     }
 
diff --git a/lib/src/backend.rs b/lib/src/backend.rs
index 2128cdde77..87a7f28dbd 100644
--- a/lib/src/backend.rs
+++ b/lib/src/backend.rs
@@ -25,6 +25,7 @@ use futures::stream::BoxStream;
 use thiserror::Error;
 
 use crate::content_hash::ContentHash;
+use crate::hex_util::to_reverse_hex_digit;
 use crate::index::Index;
 use crate::merge::Merge;
 use crate::object_id::id_type;
@@ -50,6 +51,18 @@ id_type!(pub FileId);
 id_type!(pub SymlinkId);
 id_type!(pub ConflictId);
 
+impl ChangeId {
+    pub fn reverse_hex(&self) -> String {
+        let mut buffer = vec![0; self.0.len() * 2];
+        faster_hex::hex_encode(&self.0, &mut buffer).unwrap();
+        buffer
+            .iter_mut()
+            .for_each(|b| *b = to_reverse_hex_digit(*b));
+
+        String::from_utf8(buffer).unwrap()
+    }
+}
+
 #[derive(ContentHash, Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)]
 pub struct MillisSinceEpoch(pub i64);
 
diff --git a/lib/src/hex_util.rs b/lib/src/hex_util.rs
index b1a22e8514..255a4067ce 100644
--- a/lib/src/hex_util.rs
+++ b/lib/src/hex_util.rs
@@ -14,7 +14,17 @@
 
 #![allow(missing_docs)]
 
-fn to_reverse_hex_digit(b: u8) -> Option<u8> {
+pub fn to_reverse_hex_digit(b: u8) -> u8 {
+    let offset = match b {
+        b'0'..=b'9' => b - b'0',
+        b'A'..=b'F' => b - b'A' + 10,
+        b'a'..=b'f' => b - b'a' + 10,
+        b => return b,
+    };
+    b'z' - offset
+}
+
+fn to_reverse_hex_digit_checked(b: u8) -> Option<u8> {
     let value = match b {
         b'0'..=b'9' => b - b'0',
         b'A'..=b'F' => b - b'A' + 10,
@@ -24,7 +34,7 @@ fn to_reverse_hex_digit(b: u8) -> Option<u8> {
     Some(b'z' - value)
 }
 
-fn to_forward_hex_digit(b: u8) -> Option<u8> {
+fn to_forward_hex_digit_checked(b: u8) -> Option<u8> {
     let value = match b {
         b'k'..=b'z' => b'z' - b,
         b'K'..=b'Z' => b'Z' - b,
@@ -40,14 +50,14 @@ fn to_forward_hex_digit(b: u8) -> Option<u8> {
 pub fn to_forward_hex(reverse_hex: &str) -> Option<String> {
     reverse_hex
         .bytes()
-        .map(|b| to_forward_hex_digit(b).map(char::from))
+        .map(|b| to_forward_hex_digit_checked(b).map(char::from))
         .collect()
 }
 
 pub fn to_reverse_hex(forward_hex: &str) -> Option<String> {
     forward_hex
         .bytes()
-        .map(|b| to_reverse_hex_digit(b).map(char::from))
+        .map(|b| to_reverse_hex_digit_checked(b).map(char::from))
         .collect()
 }