Skip to content

Commit becbd8d

Browse files
committed
feat!: represent object cache configuration like GITOXIDE_PACK_CACHE_MEMORY in git-configuration.
That way there is a unified system for how to set values, which may be overridable by configuration variables or not. With this changes, the explicit application of environment variables for setting the cache isn't required anymore as everything happens using git-configuration, and automatically, while providing full control like before.
1 parent a4ac9cf commit becbd8d

File tree

12 files changed

+151
-86
lines changed

12 files changed

+151
-86
lines changed

Diff for: git-repository/examples/stats.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use git_repository as git;
22
use git_repository::Reference;
33

44
fn main() -> Result<(), Box<dyn std::error::Error>> {
5-
let mut repo = git::discover(".")?.apply_environment();
5+
let mut repo = git::discover(".")?;
66
println!("Repo: {}", repo.work_dir().unwrap_or_else(|| repo.git_dir()).display());
77
let mut max_commit_size = 0;
88
let mut avg_commit_size = 0;

Diff for: git-repository/src/config/cache/init.rs

+35-5
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ impl Cache {
3232
ssh_prefix: _,
3333
http_transport,
3434
identity,
35+
gitoxide_prefix,
3536
}: repository::permissions::Environment,
3637
repository::permissions::Config {
3738
git_binary: use_installation,
@@ -133,7 +134,7 @@ impl Cache {
133134
source: git_config::Source::Api,
134135
})?;
135136
}
136-
apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity)?;
137+
apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity, gitoxide_prefix)?;
137138
globals
138139
};
139140

@@ -144,12 +145,16 @@ impl Cache {
144145
let ignore_case = config_bool(&config, "core.ignoreCase", false, lenient_config)?;
145146
let use_multi_pack_index = config_bool(&config, "core.multiPackIndex", true, lenient_config)?;
146147
let object_kind_hint = util::disambiguate_hint(&config);
148+
let (pack_cache_bytes, object_cache_bytes) =
149+
util::parse_object_caches(&config, lenient_config, filter_config_section)?;
147150
// NOTE: When adding a new initial cache, consider adjusting `reread_values_and_clear_caches()` as well.
148151
Ok(Cache {
149152
resolved: config.into(),
150153
use_multi_pack_index,
151154
object_hash,
152155
object_kind_hint,
156+
pack_cache_bytes,
157+
object_cache_bytes,
153158
reflog,
154159
is_bare,
155160
ignore_case,
@@ -203,6 +208,8 @@ impl Cache {
203208
self.personas = Default::default();
204209
self.url_rewrite = Default::default();
205210
self.diff_algorithm = Default::default();
211+
(self.pack_cache_bytes, self.object_cache_bytes) =
212+
util::parse_object_caches(config, self.lenient_config, self.filter_config_section)?;
206213
#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
207214
{
208215
self.url_scheme = Default::default();
@@ -244,6 +251,7 @@ fn apply_environment_overrides(
244251
git_prefix: Permission,
245252
http_transport: Permission,
246253
identity: Permission,
254+
gitoxide_prefix: Permission,
247255
) -> Result<(), Error> {
248256
fn var_as_bstring(var: &str, perm: Permission) -> Option<BString> {
249257
perm.check_opt(var)
@@ -418,11 +426,33 @@ fn apply_environment_overrides(
418426
.new_section("gitoxide", Some(Cow::Borrowed("objects".into())))
419427
.expect("statically known valid section name");
420428

421-
for (var, key) in [
422-
("GIT_NO_REPLACE_OBJECTS", "noReplace"),
423-
("GIT_REPLACE_REF_BASE", "replaceRefBase"),
429+
for (var, key, permission) in [
430+
("GIT_NO_REPLACE_OBJECTS", "noReplace", git_prefix),
431+
("GIT_REPLACE_REF_BASE", "replaceRefBase", git_prefix),
432+
("GITOXIDE_OBJECT_CACHE_MEMORY", "cacheLimit", gitoxide_prefix),
424433
] {
425-
if let Some(value) = var_as_bstring(var, git_prefix) {
434+
if let Some(value) = var_as_bstring(var, permission) {
435+
section.push_with_comment(
436+
key.try_into().expect("statically known to be valid"),
437+
Some(value.as_ref()),
438+
format!("from {var}").as_str(),
439+
);
440+
}
441+
}
442+
443+
if section.num_values() == 0 {
444+
let id = section.id();
445+
env_override.remove_section_by_id(id);
446+
}
447+
}
448+
449+
{
450+
let mut section = env_override
451+
.new_section("core", None)
452+
.expect("statically known valid section name");
453+
454+
for (var, key) in [("GITOXIDE_PACK_CACHE_MEMORY", "deltaBaseCacheLimit")] {
455+
if let Some(value) = var_as_bstring(var, gitoxide_prefix) {
426456
section.push_with_comment(
427457
key.try_into().expect("statically known to be valid"),
428458
Some(value.as_ref()),

Diff for: git-repository/src/config/cache/util.rs

+25
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,31 @@ pub(crate) fn reflog_or_default(
104104
})
105105
}
106106

107+
/// Return `(pack_cache_bytes, object_cache_bytes)` as parsed from git-config
108+
pub(crate) fn parse_object_caches(
109+
config: &git_config::File<'static>,
110+
lenient: bool,
111+
mut filter_config_section: fn(&git_config::file::Metadata) -> bool,
112+
) -> Result<(Option<usize>, usize), Error> {
113+
let key = "core.deltaBaseCacheLimit";
114+
let pack_cache_bytes = config
115+
.integer_filter_by_key(key, &mut filter_config_section)
116+
.transpose()
117+
.with_leniency(lenient)
118+
.map_err(|err| Error::Value { source: err, key })?;
119+
let key = "gitoxide.objects.cacheLimit";
120+
let object_cache_bytes = config
121+
.integer_filter_by_key(key, &mut filter_config_section)
122+
.transpose()
123+
.with_leniency(lenient)
124+
.map_err(|err| Error::Value { source: err, key })?
125+
.unwrap_or_default();
126+
Ok((
127+
pack_cache_bytes.and_then(|v| v.try_into().ok()),
128+
object_cache_bytes.try_into().unwrap_or_default(),
129+
))
130+
}
131+
107132
pub(crate) fn parse_core_abbrev(
108133
config: &git_config::File<'static>,
109134
object_hash: git_hash::Kind,

Diff for: git-repository/src/config/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@ pub(crate) struct Cache {
195195
pub(crate) url_scheme: OnceCell<remote::url::SchemePermission>,
196196
/// The algorithm to use when diffing blobs
197197
pub(crate) diff_algorithm: OnceCell<git_diff::blob::Algorithm>,
198+
/// The amount of bytes to use for a memory backed delta pack cache. If `Some(0)`, no cache is used, if `None`
199+
/// a standard cache is used which costs near to nothing and always pays for itself.
200+
pub(crate) pack_cache_bytes: Option<usize>,
201+
/// The amount of bytes to use for caching whole objects, or 0 to turn it off entirely.
202+
pub(crate) object_cache_bytes: usize,
198203
/// The config section filter from the options used to initialize this instance. Keep these in sync!
199204
filter_config_section: fn(&git_config::file::Metadata) -> bool,
200205
/// The object kind to pick if a prefix is ambiguous.

Diff for: git-repository/src/lib.rs

-3
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@
3434
//! Use the `cache-efficiency-debug` cargo feature to learn how efficient the cache actually is - it's easy to end up with lowered
3535
//! performance if the cache is not hit in 50% of the time.
3636
//!
37-
//! Environment variables can also be used for configuration if the application is calling
38-
//! [`apply_environment()`][crate::Repository::apply_environment()].
39-
//!
4037
//! ### Shortcomings & Limitations
4138
//!
4239
//! - Only a single `crate::object` or derivatives can be held in memory at a time, _per `Easy*`_.

Diff for: git-repository/src/repository/cache.rs

-63
Original file line numberDiff line numberDiff line change
@@ -26,67 +26,4 @@ impl crate::Repository {
2626
self.object_cache_size(bytes)
2727
}
2828
}
29-
30-
/// Read well-known environment variables related to caches and apply them to this instance, but not to clones of it - each
31-
/// needs their own configuration.
32-
///
33-
/// Note that environment configuration never fails due to invalid environment values, but it should be used with caution as it
34-
/// could be used to cause high memory consumption.
35-
///
36-
/// Use the `GITOXIDE_DISABLE_PACK_CACHE` environment variable to turn off any pack cache, which can be beneficial when it's known that
37-
/// the cache efficiency is low. Use `GITOXIDE_PACK_CACHE_MEMORY=512MB` to use up to 512MB of RAM for the pack delta base
38-
/// cache. If none of these are set, the default cache is fast enough to nearly never cause a (marginal) slow-down while providing
39-
/// some gains most of the time. Note that the value given is _per-thread_.
40-
///
41-
/// Use the `GITOXIDE_OBJECT_CACHE_MEMORY=16mb` to set the given amount of memory to store full objects, on a per-thread basis.
42-
pub fn apply_environment(self) -> Self {
43-
// We have no cache types available without this flag currently. Maybe this should change at some point.
44-
#[cfg(not(feature = "max-performance-safe"))]
45-
return self;
46-
#[cfg(feature = "max-performance-safe")]
47-
{
48-
let pack_cache_disabled = std::env::var_os("GITOXIDE_DISABLE_PACK_CACHE").is_some();
49-
let mut this = self;
50-
if !pack_cache_disabled {
51-
let bytes = parse_bytes_from_var("GITOXIDE_PACK_CACHE_MEMORY");
52-
let new_pack_cache = move || -> Box<git_odb::cache::PackCache> {
53-
match bytes {
54-
Some(bytes) => Box::new(git_pack::cache::lru::MemoryCappedHashmap::new(bytes)),
55-
None => Box::new(git_pack::cache::lru::StaticLinkedList::<64>::default()),
56-
}
57-
};
58-
this.objects.set_pack_cache(new_pack_cache);
59-
} else {
60-
this.objects.unset_pack_cache();
61-
}
62-
63-
if let Some(bytes) = parse_bytes_from_var("GITOXIDE_OBJECT_CACHE_MEMORY") {
64-
this.objects
65-
.set_object_cache(move || Box::new(git_pack::cache::object::MemoryCappedHashmap::new(bytes)));
66-
}
67-
this
68-
}
69-
}
70-
}
71-
72-
#[cfg(feature = "max-performance-safe")]
73-
fn parse_bytes_from_var(name: &str) -> Option<usize> {
74-
std::env::var(name)
75-
.ok()
76-
.and_then(|v| {
77-
byte_unit::Byte::from_str(&v)
78-
.map_err(|err| log::warn!("Failed to parse {:?} into byte unit for pack cache: {}", v, err))
79-
.ok()
80-
})
81-
.and_then(|unit| {
82-
unit.get_bytes()
83-
.try_into()
84-
.map_err(|err| {
85-
log::warn!(
86-
"Parsed bytes value is not representable as usize. Defaulting to standard pack cache: {}",
87-
err
88-
)
89-
})
90-
.ok()
91-
})
9229
}

Diff for: git-repository/src/repository/init.rs

+27-10
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,12 @@ impl crate::Repository {
1010
linked_worktree_options: crate::open::Options,
1111
index: crate::worktree::IndexStorage,
1212
) -> Self {
13+
let objects = setup_objects(objects, &config);
1314
crate::Repository {
1415
bufs: RefCell::new(Vec::with_capacity(4)),
1516
work_tree,
1617
common_dir,
17-
objects: {
18-
#[cfg(feature = "max-performance-safe")]
19-
{
20-
objects.with_pack_cache(|| Box::new(git_pack::cache::lru::StaticLinkedList::<64>::default()))
21-
}
22-
#[cfg(not(feature = "max-performance-safe"))]
23-
{
24-
objects
25-
}
26-
},
18+
objects,
2719
refs,
2820
config,
2921
options: linked_worktree_options,
@@ -36,3 +28,28 @@ impl crate::Repository {
3628
self.into()
3729
}
3830
}
31+
32+
#[cfg_attr(not(feature = "max-performance-safe"), allow(unused_variables, unused_mut))]
33+
fn setup_objects(mut objects: crate::OdbHandle, config: &crate::config::Cache) -> crate::OdbHandle {
34+
#[cfg(feature = "max-performance-safe")]
35+
{
36+
match config.pack_cache_bytes {
37+
None => objects.set_pack_cache(|| Box::new(git_pack::cache::lru::StaticLinkedList::<64>::default())),
38+
Some(0) => objects.unset_pack_cache(),
39+
Some(bytes) => objects.set_pack_cache(move || -> Box<git_odb::cache::PackCache> {
40+
Box::new(git_pack::cache::lru::MemoryCappedHashmap::new(bytes))
41+
}),
42+
};
43+
if config.object_cache_bytes == 0 {
44+
objects.unset_object_cache();
45+
} else {
46+
let bytes = config.object_cache_bytes;
47+
objects.set_object_cache(move || Box::new(git_pack::cache::object::MemoryCappedHashmap::new(bytes)));
48+
}
49+
objects
50+
}
51+
#[cfg(not(feature = "max-performance-safe"))]
52+
{
53+
objects
54+
}
55+
}

Diff for: git-repository/src/repository/permissions.rs

+4
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ pub struct Environment {
8282
/// Note that identity related environment variables prefixed with `GIT_` are falling under the
8383
/// `git_prefix` permission, like `GIT_AUTHOR_NAME`.
8484
pub identity: git_sec::Permission,
85+
/// Decide if `gitoxide` specific variables may be read, prefixed with `GITOXIDE_`.
86+
pub gitoxide_prefix: git_sec::Permission,
8587
}
8688

8789
impl Environment {
@@ -94,6 +96,7 @@ impl Environment {
9496
ssh_prefix: git_sec::Permission::Allow,
9597
http_transport: git_sec::Permission::Allow,
9698
identity: git_sec::Permission::Allow,
99+
gitoxide_prefix: git_sec::Permission::Allow,
97100
}
98101
}
99102
}
@@ -140,6 +143,7 @@ impl Permissions {
140143
git_prefix: deny,
141144
http_transport: deny,
142145
identity: deny,
146+
gitoxide_prefix: deny,
143147
}
144148
},
145149
}
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
version https://git-lfs.github.com/spec/v1
2-
oid sha256:607f9cd2af225da2cff5640855d557b363949bd667454e772ae49a63deb21831
3-
size 13284
2+
oid sha256:3d893d90b1870819d92b25f84214d10e7565f6cab71032959659f284642dc5a4
3+
size 13808

Diff for: git-repository/tests/fixtures/make_config_repos.sh

+12
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,15 @@ git init http-proxy-authenticated
9999
followRedirects
100100
EOF
101101
)
102+
103+
git init object-caches
104+
(cd object-caches
105+
git config core.deltaBaseCacheLimit 128m
106+
git config gitoxide.objects.cacheLimit 16m
107+
)
108+
109+
git init disabled-object-caches
110+
(cd disabled-object-caches
111+
git config core.deltaBaseCacheLimit 0
112+
git config gitoxide.objects.cacheLimit 0
113+
)

Diff for: git-repository/tests/repository/open.rs

+29-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,29 @@ mod submodules {
4545
}
4646
}
4747

48+
mod object_caches {
49+
use crate::util::named_subrepo_opts;
50+
use git_repository as git;
51+
52+
#[test]
53+
fn default_git_and_custom_caches() -> crate::Result {
54+
let opts = git::open::Options::isolated();
55+
let repo = named_subrepo_opts("make_config_repos.sh", "object-caches", opts)?;
56+
assert!(repo.objects.has_object_cache());
57+
assert!(repo.objects.has_pack_cache());
58+
Ok(())
59+
}
60+
61+
#[test]
62+
fn disabled() -> crate::Result {
63+
let opts = git::open::Options::isolated();
64+
let repo = named_subrepo_opts("make_config_repos.sh", "disabled-object-caches", opts)?;
65+
assert!(!repo.objects.has_object_cache());
66+
assert!(!repo.objects.has_pack_cache());
67+
Ok(())
68+
}
69+
}
70+
4871
mod with_overrides {
4972
use crate::util::named_subrepo_opts;
5073
use git_object::bstr::BStr;
@@ -80,7 +103,9 @@ mod with_overrides {
80103
.set("GIT_AUTHOR_NAME", "author name")
81104
.set("GIT_AUTHOR_EMAIL", "author email")
82105
.set("GIT_AUTHOR_DATE", default_date)
83-
.set("EMAIL", "user email");
106+
.set("EMAIL", "user email")
107+
.set("GITOXIDE_PACK_CACHE_MEMORY", "0")
108+
.set("GITOXIDE_OBJECT_CACHE_MEMORY", "5m");
84109
let mut opts = git::open::Options::isolated()
85110
.config_overrides([
86111
"http.userAgent=agent-from-api",
@@ -95,6 +120,7 @@ mod with_overrides {
95120
opts.permissions.env.git_prefix = Permission::Allow;
96121
opts.permissions.env.http_transport = Permission::Allow;
97122
opts.permissions.env.identity = Permission::Allow;
123+
opts.permissions.env.gitoxide_prefix = Permission::Allow;
98124
let repo = named_subrepo_opts("make_config_repos.sh", "http-config", opts)?;
99125
let config = repo.config_snapshot();
100126
assert_eq!(
@@ -175,6 +201,8 @@ mod with_overrides {
175201
("gitoxide.commit.authorDate", default_date),
176202
("gitoxide.commit.committerDate", default_date),
177203
("gitoxide.user.emailFallback", "user email"),
204+
("core.deltaBaseCacheLimit", "0"),
205+
("gitoxide.objects.cacheLimit", "5m"),
178206
] {
179207
assert_eq!(
180208
config

0 commit comments

Comments
 (0)