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.

Sebastian Thiel committed 3 years ago

Commit becbd8d1 parent a4ac9cf12 files · +151 −86Browse files

Showing the changes to git-repository/src (7 of 12 files). Show all

Unified · Split

7 files

git-repository/src/config/cache/init.rs +35 −5
git-repository/src/config/cache/util.rs +25 −0
git-repository/src/config/mod.rs +5 −0
git-repository/src/lib.rs +0 −3
git-repository/src/repository/cache.rs +0 −63
git-repository/src/repository/init.rs +27 −10
git-repository/src/repository/permissions.rs +4 −0

git-repository/src/config/cache/init.rs+35−5

             ssh_prefix: _,
             http_transport,
             identity,
+            gitoxide_prefix,
         }: repository::permissions::Environment,
         repository::permissions::Config {
             git_binary: use_installation,
                         source: git_config::Source::Api,
                     })?;
             }
-            apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity)?;
+            apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity, gitoxide_prefix)?;
             globals
         };
         let ignore_case = config_bool(&config, "core.ignoreCase", false, lenient_config)?;
         let use_multi_pack_index = config_bool(&config, "core.multiPackIndex", true, lenient_config)?;
         let object_kind_hint = util::disambiguate_hint(&config);
+        let (pack_cache_bytes, object_cache_bytes) =
+            util::parse_object_caches(&config, lenient_config, filter_config_section)?;
         // NOTE: When adding a new initial cache, consider adjusting `reread_values_and_clear_caches()` as well.
         Ok(Cache {
             resolved: config.into(),
             use_multi_pack_index,
             object_hash,
             object_kind_hint,
+            pack_cache_bytes,
+            object_cache_bytes,
             reflog,
             is_bare,
             ignore_case,
         self.personas = Default::default();
         self.url_rewrite = Default::default();
         self.diff_algorithm = Default::default();
+        (self.pack_cache_bytes, self.object_cache_bytes) =
+            util::parse_object_caches(config, self.lenient_config, self.filter_config_section)?;
         #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
         {
             self.url_scheme = Default::default();
     git_prefix: Permission,
     http_transport: Permission,
     identity: Permission,
+    gitoxide_prefix: Permission,
 ) -> Result<(), Error> {
     fn var_as_bstring(var: &str, perm: Permission) -> Option<BString> {
         perm.check_opt(var)
             .new_section("gitoxide", Some(Cow::Borrowed("objects".into())))
             .expect("statically known valid section name");
-        for (var, key) in [
+        for (var, key, permission) in [
-            ("GIT_NO_REPLACE_OBJECTS", "noReplace"),
+            ("GIT_NO_REPLACE_OBJECTS", "noReplace", git_prefix),
-            ("GIT_REPLACE_REF_BASE", "replaceRefBase"),
+            ("GIT_REPLACE_REF_BASE", "replaceRefBase", git_prefix),
+            ("GITOXIDE_OBJECT_CACHE_MEMORY", "cacheLimit", gitoxide_prefix),
         ] {
-            if let Some(value) = var_as_bstring(var, git_prefix) {
+            if let Some(value) = var_as_bstring(var, permission) {
+                section.push_with_comment(
+                    key.try_into().expect("statically known to be valid"),
+                    Some(value.as_ref()),
+                    format!("from {var}").as_str(),
+                );
+            }
+        }
+        if section.num_values() == 0 {
+            let id = section.id();
+            env_override.remove_section_by_id(id);
+        }
+    }
+    {
+        let mut section = env_override
+            .new_section("core", None)
+            .expect("statically known valid section name");
+        for (var, key) in [("GITOXIDE_PACK_CACHE_MEMORY", "deltaBaseCacheLimit")] {
+            if let Some(value) = var_as_bstring(var, gitoxide_prefix) {
                 section.push_with_comment(
                     key.try_into().expect("statically known to be valid"),
                     Some(value.as_ref()),

git-repository/src/config/cache/util.rs+25−0

@@ -104,6 +104,31 @@ pub(crate) fn reflog_or_default( · 103 unmodified lines
104	})	104	})
105	}	105	}
106		106
		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
107	pub(crate) fn parse_core_abbrev(	132	pub(crate) fn parse_core_abbrev(
108	config: &git_config::File<'static>,	133	config: &git_config::File<'static>,
109	object_hash: git_hash::Kind,	134	object_hash: git_hash::Kind,

git-repository/src/config/mod.rs+5−0

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

git-repository/src/lib.rs+0−3

 //! Use the `cache-efficiency-debug` cargo feature to learn how efficient the cache actually is - it's easy to end up with lowered
 //! performance if the cache is not hit in 50% of the time.
 //!
-//! Environment variables can also be used for configuration if the application is calling
-//! [`apply_environment()`][crate::Repository::apply_environment()].
-//!
 //! ### Shortcomings & Limitations
 //!
 //! - Only a single `crate::object` or derivatives can be held in memory at a time, _per `Easy*`_.

git-repository/src/repository/cache.rs+0−63

@@ -26,67 +26,4 @@ impl crate::Repository { · 25 unmodified lines
26	self.object_cache_size(bytes)	26	self.object_cache_size(bytes)
27	}	27	}
28	}	28	}
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	})
92	}	29	}

git-repository/src/repository/init.rs+27−10

@@ -10,20 +10,12 @@ impl crate::Repository { · 9 unmodified lines
10	linked_worktree_options: crate::open::Options,	10	linked_worktree_options: crate::open::Options,
11	index: crate::worktree::IndexStorage,	11	index: crate::worktree::IndexStorage,
12	) -> Self {	12	) -> Self {
		13	let objects = setup_objects(objects, &config);
13	crate::Repository {	14	crate::Repository {
14	bufs: RefCell::new(Vec::with_capacity(4)),	15	bufs: RefCell::new(Vec::with_capacity(4)),
15	work_tree,	16	work_tree,
16	common_dir,	17	common_dir,
17	objects: {	18	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	},
27	refs,	19	refs,
28	config,	20	config,
29	options: linked_worktree_options,	21	options: linked_worktree_options,
@@ -36,3 +28,28 @@ impl crate::Repository { · 6 unmodified lines
36	self.into()	28	self.into()
37	}	29	}
38	}	30	}
		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	}

git-repository/src/repository/permissions.rs+4−0

     /// Note that identity related environment variables prefixed with `GIT_` are falling under the
     /// `git_prefix` permission, like `GIT_AUTHOR_NAME`.
     pub identity: git_sec::Permission,
+    /// Decide if `gitoxide` specific variables may be read, prefixed with `GITOXIDE_`.
+    pub gitoxide_prefix: git_sec::Permission,
 }
 impl Environment {
             ssh_prefix: git_sec::Permission::Allow,
             http_transport: git_sec::Permission::Allow,
             identity: git_sec::Permission::Allow,
+            gitoxide_prefix: git_sec::Permission::Allow,
         }
     }
 }
                     git_prefix: deny,
                     http_transport: deny,
                     identity: deny,
+                    gitoxide_prefix: deny,
                 }
             },
         }