ok/jj
1
0
Fork 0
forked from mirrors/jj

repo_path: split RepoPathComponent into owned and borrowed types

This is a step towards introducing a borrowed RepoPath type. The current
RepoPath type is inefficient as each component String is usually short. We
could apply short-string optimization, but still each inlined component would
consume 24 bytes just for e.g. "src", and increase the chance of random memory
access. If the owned RepoPath type is backed by String, we can implement cheap
cast from &str to borrowed &RepoPath type.
This commit is contained in:
Yuya Nishihara 2023-11-25 18:22:09 +09:00
parent f2096da2d6
commit 59ef3f0023
7 changed files with 125 additions and 64 deletions

View file

@ -384,7 +384,7 @@ impl Tree {
}
pub fn names(&self) -> impl Iterator<Item = &RepoPathComponent> {
self.entries.keys()
self.entries.keys().map(|name| name.as_ref())
}
pub fn entries(&self) -> TreeEntriesNonRecursiveIterator {
@ -407,7 +407,7 @@ impl Tree {
self.entries.remove(name);
}
Some(value) => {
self.entries.insert(name.clone(), value);
self.entries.insert(name.to_owned(), value);
}
}
}

View file

@ -783,7 +783,7 @@ impl TreeState {
if name == ".jj" || name == ".git" {
return Ok(());
}
let path = dir.join(&RepoPathComponent::from(name));
let path = dir.join(RepoPathComponent::new(name));
if let Some(file_state) = file_states.get(&path) {
if file_state.file_type == FileType::GitSubmodule {
return Ok(());

View file

@ -14,25 +14,46 @@
#![allow(missing_docs)]
use std::borrow::Borrow;
use std::fmt::{Debug, Error, Formatter};
use std::ops::Deref;
use std::path::{Component, Path, PathBuf};
use itertools::Itertools;
use ref_cast::{ref_cast_custom, RefCastCustom};
use thiserror::Error;
use crate::file_util;
// TODO: make RepoPathComponent a borrowed type
pub type RepoPathComponentBuf = RepoPathComponent;
content_hash! {
/// Owned `RepoPath` component.
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
pub struct RepoPathComponent {
pub struct RepoPathComponentBuf {
// Don't add more fields. Eq, Hash, and Ord must be compatible with the
// borrowed RepoPathComponent type.
value: String,
}
}
/// Borrowed `RepoPath` component.
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Hash, RefCastCustom)]
#[repr(transparent)]
pub struct RepoPathComponent {
value: str,
}
impl RepoPathComponent {
/// Wraps `value` as `RepoPathComponent`.
///
/// The input `value` must not be empty and not contain path separator.
pub fn new(value: &str) -> &Self {
assert!(is_valid_repo_path_component_str(value));
Self::new_unchecked(value)
}
#[ref_cast_custom]
const fn new_unchecked(value: &str) -> &Self;
pub fn as_str(&self) -> &str {
&self.value
}
@ -51,6 +72,45 @@ impl From<String> for RepoPathComponentBuf {
}
}
impl AsRef<RepoPathComponent> for RepoPathComponent {
fn as_ref(&self) -> &RepoPathComponent {
self
}
}
impl AsRef<RepoPathComponent> for RepoPathComponentBuf {
fn as_ref(&self) -> &RepoPathComponent {
self
}
}
impl Borrow<RepoPathComponent> for RepoPathComponentBuf {
fn borrow(&self) -> &RepoPathComponent {
self
}
}
impl Deref for RepoPathComponentBuf {
type Target = RepoPathComponent;
fn deref(&self) -> &Self::Target {
RepoPathComponent::new_unchecked(&self.value)
}
}
impl ToOwned for RepoPathComponent {
type Owned = RepoPathComponentBuf;
fn to_owned(&self) -> Self::Owned {
let value = self.value.to_owned();
RepoPathComponentBuf { value }
}
fn clone_into(&self, target: &mut Self::Owned) {
self.value.clone_into(&mut target.value);
}
}
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct RepoPath {
components: Vec<RepoPathComponentBuf>,
@ -189,7 +249,8 @@ impl RepoPath {
}
pub fn join(&self, entry: &RepoPathComponent) -> RepoPath {
let components = self.components.iter().chain([entry]).cloned().collect();
let components =
itertools::chain(self.components.iter().cloned(), [entry.to_owned()]).collect();
RepoPath { components }
}
}
@ -259,12 +320,12 @@ mod tests {
#[test]
fn test_join() {
let root = RepoPath::root();
let dir = root.join(&RepoPathComponent::from("dir"));
let dir = root.join(RepoPathComponent::new("dir"));
assert_eq!(dir, repo_path("dir"));
let subdir = dir.join(&RepoPathComponent::from("subdir"));
let subdir = dir.join(RepoPathComponent::new("subdir"));
assert_eq!(subdir, repo_path("dir/subdir"));
assert_eq!(
subdir.join(&RepoPathComponent::from("file")),
subdir.join(RepoPathComponent::new("file")),
repo_path("dir/subdir/file")
);
}
@ -272,8 +333,8 @@ mod tests {
#[test]
fn test_parent() {
let root = RepoPath::root();
let dir_component = &RepoPathComponent::from("dir");
let subdir_component = &RepoPathComponent::from("subdir");
let dir_component = RepoPathComponent::new("dir");
let subdir_component = RepoPathComponent::new("subdir");
let dir = root.join(dir_component);
let subdir = dir.join(subdir_component);
@ -286,8 +347,8 @@ mod tests {
#[test]
fn test_split() {
let root = RepoPath::root();
let dir_component = &RepoPathComponent::from("dir");
let file_component = &RepoPathComponent::from("file");
let dir_component = RepoPathComponent::new("dir");
let file_component = RepoPathComponent::new("file");
let dir = root.join(dir_component);
let file = dir.join(file_component);

View file

@ -82,7 +82,7 @@ impl TreeBuilder {
let tree = trees_to_write.get_mut(&dir).unwrap();
match file_override {
Override::Replace(value) => {
tree.set(basename.clone(), value);
tree.set(basename.to_owned(), value);
}
Override::Tombstone => {
tree.remove(basename);
@ -104,7 +104,7 @@ impl TreeBuilder {
}
} else {
let tree = store.write_tree(&dir, tree).unwrap();
parent_tree.set(basename.clone(), TreeValue::Tree(tree.id().clone()));
parent_tree.set(basename.to_owned(), TreeValue::Tree(tree.id().clone()));
}
} else {
// We're writing the root tree. Write it even if empty. Return its id.

View file

@ -145,7 +145,7 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) {
Merge::normal(TreeValue::Symlink(id))
}
Kind::Tree => {
let file_path = path.join(&RepoPathComponent::from("file"));
let file_path = path.join(RepoPathComponent::new("file"));
let id = testutils::write_file(store, &file_path, "normal file contents");
let value = TreeValue::File {
id,
@ -348,7 +348,7 @@ fn test_conflicting_changes_on_disk() {
&[
(&file_file_path, "committed contents"),
(
&file_dir_path.join(&RepoPathComponent::from("file")),
&file_dir_path.join(RepoPathComponent::new("file")),
"committed contents",
),
(&dir_file_path, "committed contents"),

View file

@ -83,36 +83,36 @@ fn test_same_type() {
// Check that the simple, non-conflicting cases were resolved correctly
assert_eq!(
merged_tree.value(&RepoPathComponent::from("__a")),
side2_tree.value(&RepoPathComponent::from("__a"))
merged_tree.value(RepoPathComponent::new("__a")),
side2_tree.value(RepoPathComponent::new("__a"))
);
assert_eq!(
merged_tree.value(&RepoPathComponent::from("_a_")),
side1_tree.value(&RepoPathComponent::from("_a_"))
merged_tree.value(RepoPathComponent::new("_a_")),
side1_tree.value(RepoPathComponent::new("_a_"))
);
assert_eq!(
merged_tree.value(&RepoPathComponent::from("_aa")),
side1_tree.value(&RepoPathComponent::from("_aa"))
merged_tree.value(RepoPathComponent::new("_aa")),
side1_tree.value(RepoPathComponent::new("_aa"))
);
assert_eq!(
merged_tree.value(&RepoPathComponent::from("aaa")),
side1_tree.value(&RepoPathComponent::from("aaa"))
merged_tree.value(RepoPathComponent::new("aaa")),
side1_tree.value(RepoPathComponent::new("aaa"))
);
assert_eq!(
merged_tree.value(&RepoPathComponent::from("aab")),
side2_tree.value(&RepoPathComponent::from("aab"))
merged_tree.value(RepoPathComponent::new("aab")),
side2_tree.value(RepoPathComponent::new("aab"))
);
assert_eq!(
merged_tree.value(&RepoPathComponent::from("aba")),
side1_tree.value(&RepoPathComponent::from("aba"))
merged_tree.value(RepoPathComponent::new("aba")),
side1_tree.value(RepoPathComponent::new("aba"))
);
assert_eq!(
merged_tree.value(&RepoPathComponent::from("abb")),
side1_tree.value(&RepoPathComponent::from("abb"))
merged_tree.value(RepoPathComponent::new("abb")),
side1_tree.value(RepoPathComponent::new("abb"))
);
// Check the conflicting cases
let component = &RepoPathComponent::from("_ab");
let component = RepoPathComponent::new("_ab");
match merged_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
@ -129,7 +129,7 @@ fn test_same_type() {
}
_ => panic!("unexpected value"),
};
let component = &RepoPathComponent::from("a_b");
let component = RepoPathComponent::new("a_b");
match merged_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
@ -146,7 +146,7 @@ fn test_same_type() {
}
_ => panic!("unexpected value"),
};
let component = &RepoPathComponent::from("ab_");
let component = RepoPathComponent::new("ab_");
match merged_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
@ -163,7 +163,7 @@ fn test_same_type() {
}
_ => panic!("unexpected value"),
};
let component = &RepoPathComponent::from("abc");
let component = RepoPathComponent::new("abc");
match merged_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
@ -221,16 +221,16 @@ fn test_executable() {
let merged_tree = merge_trees(&side1_tree, &base_tree, &side2_tree).unwrap();
// Check that the merged tree has the correct executable bits
let norm = base_tree.value(&RepoPathComponent::from("nnn"));
let exec = base_tree.value(&RepoPathComponent::from("xxx"));
assert_eq!(merged_tree.value(&RepoPathComponent::from("nnn")), norm);
assert_eq!(merged_tree.value(&RepoPathComponent::from("nnx")), exec);
assert_eq!(merged_tree.value(&RepoPathComponent::from("nxn")), exec);
assert_eq!(merged_tree.value(&RepoPathComponent::from("nxx")), exec);
assert_eq!(merged_tree.value(&RepoPathComponent::from("xnn")), norm);
assert_eq!(merged_tree.value(&RepoPathComponent::from("xnx")), norm);
assert_eq!(merged_tree.value(&RepoPathComponent::from("xxn")), norm);
assert_eq!(merged_tree.value(&RepoPathComponent::from("xxx")), exec);
let norm = base_tree.value(RepoPathComponent::new("nnn"));
let exec = base_tree.value(RepoPathComponent::new("xxx"));
assert_eq!(merged_tree.value(RepoPathComponent::new("nnn")), norm);
assert_eq!(merged_tree.value(RepoPathComponent::new("nnx")), exec);
assert_eq!(merged_tree.value(RepoPathComponent::new("nxn")), exec);
assert_eq!(merged_tree.value(RepoPathComponent::new("nxx")), exec);
assert_eq!(merged_tree.value(RepoPathComponent::new("xnn")), norm);
assert_eq!(merged_tree.value(RepoPathComponent::new("xnx")), norm);
assert_eq!(merged_tree.value(RepoPathComponent::new("xxn")), norm);
assert_eq!(merged_tree.value(RepoPathComponent::new("xxx")), exec);
}
#[test]
@ -411,7 +411,7 @@ fn test_types() {
let merged_tree = merge_trees(&side1_tree, &base_tree, &side2_tree).unwrap();
// Check the conflicting cases
let component = &RepoPathComponent::from("normal_executable_symlink");
let component = RepoPathComponent::new("normal_executable_symlink");
match merged_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
@ -431,7 +431,7 @@ fn test_types() {
}
_ => panic!("unexpected value"),
};
let component = &RepoPathComponent::from("tree_normal_symlink");
let component = RepoPathComponent::new("tree_normal_symlink");
match merged_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
@ -456,7 +456,7 @@ fn test_simplify_conflict() {
let repo = &test_repo.repo;
let store = repo.store();
let component = &RepoPathComponent::from("file");
let component = RepoPathComponent::new("file");
let path = RepoPath::from_internal_string("file");
let write_tree = |contents: &str| -> Tree { create_single_tree(repo, &[(&path, contents)]) };
@ -495,7 +495,7 @@ fn test_simplify_conflict() {
match further_rebased_tree.value(component).unwrap() {
TreeValue::Conflict(id) => {
let conflict = store
.read_conflict(&RepoPath::from_components(vec![component.clone()]), id)
.read_conflict(&RepoPath::from_components(vec![component.to_owned()]), id)
.unwrap();
assert_eq!(
conflict.removes().map(|v| v.as_ref()).collect_vec(),

View file

@ -123,10 +123,10 @@ fn test_from_legacy_tree() {
tree_builder.set(file5_path.clone(), TreeValue::Conflict(file5_conflict_id));
// dir1: directory without conflicts
let dir1_basename = &RepoPathComponent::from("dir1");
let dir1_basename = RepoPathComponent::new("dir1");
let dir1_filename = RepoPath::root()
.join(dir1_basename)
.join(&RepoPathComponent::from("file"));
.join(RepoPathComponent::new("file"));
let dir1_filename_id = write_file(store.as_ref(), &dir1_filename, "file5_v2");
tree_builder.set(dir1_filename.clone(), file_value(&dir1_filename_id));
@ -135,7 +135,7 @@ fn test_from_legacy_tree() {
let merged_tree = MergedTree::from_legacy_tree(tree.clone()).unwrap();
assert_eq!(
merged_tree.value(&RepoPathComponent::from("missing")),
merged_tree.value(RepoPathComponent::new("missing")),
MergedTreeVal::Resolved(None)
);
// file1: regular file without conflicts
@ -373,11 +373,11 @@ fn test_resolve_success() {
let trivial_file_path = RepoPath::from_internal_string("trivial-file");
let trivial_hunk_path = RepoPath::from_internal_string("trivial-hunk");
let both_added_dir_path = RepoPath::from_internal_string("added-dir");
let both_added_dir_file1_path = both_added_dir_path.join(&RepoPathComponent::from("file1"));
let both_added_dir_file2_path = both_added_dir_path.join(&RepoPathComponent::from("file2"));
let both_added_dir_file1_path = both_added_dir_path.join(RepoPathComponent::new("file1"));
let both_added_dir_file2_path = both_added_dir_path.join(RepoPathComponent::new("file2"));
let emptied_dir_path = RepoPath::from_internal_string("to-become-empty");
let emptied_dir_file1_path = emptied_dir_path.join(&RepoPathComponent::from("file1"));
let emptied_dir_file2_path = emptied_dir_path.join(&RepoPathComponent::from("file2"));
let emptied_dir_file1_path = emptied_dir_path.join(RepoPathComponent::new("file1"));
let emptied_dir_file2_path = emptied_dir_path.join(RepoPathComponent::new("file2"));
let base1 = create_single_tree(
repo,
&[
@ -503,7 +503,7 @@ fn test_conflict_iterator() {
(&dir_file_path, "base"),
// no added_dir_path
(
&modify_delete_dir_path.join(&RepoPathComponent::from("base")),
&modify_delete_dir_path.join(RepoPathComponent::new("base")),
"base",
),
],
@ -520,11 +520,11 @@ fn test_conflict_iterator() {
(&different_add_path, "side1"),
(&dir_file_path, "side1"),
(
&added_dir_path.join(&RepoPathComponent::from("side1")),
&added_dir_path.join(RepoPathComponent::new("side1")),
"side1",
),
(
&modify_delete_dir_path.join(&RepoPathComponent::from("side1")),
&modify_delete_dir_path.join(RepoPathComponent::new("side1")),
"side1",
),
],
@ -539,9 +539,9 @@ fn test_conflict_iterator() {
// no modify_delete_path
(&same_add_path, "same"),
(&different_add_path, "side2"),
(&dir_file_path.join(&RepoPathComponent::from("dir")), "new"),
(&dir_file_path.join(RepoPathComponent::new("dir")), "new"),
(
&added_dir_path.join(&RepoPathComponent::from("side2")),
&added_dir_path.join(RepoPathComponent::new("side2")),
"side2",
),
// no modify_delete_dir_path
@ -859,7 +859,7 @@ fn test_diff_dir_file() {
let path4 = RepoPath::from_internal_string("path4");
let path5 = RepoPath::from_internal_string("path5");
let path6 = RepoPath::from_internal_string("path6");
let file = &RepoPathComponent::from("file");
let file = RepoPathComponent::new("file");
let left_base = create_single_tree(
repo,
&[