Re: [PATCH 01/16] odb: introduce "inmemory" source

[Justin Tobler <jltobler@gmail.com>]

On 26/04/03 08:01AM, Patrick Steinhardt wrote:
> Next to our typical object database sources, each object database also
> has an implicit source of "cached" objects. These cached objects only
> exist in memory and some use cases:
> 
>   - They contain evergreen objects that we expect to always exist, like
>     for example the empty tree.
> 
>   - They can be used to store temporary objects that we don't want to
>     persist to disk.
> 
> Overall, their use is somewhat restricted though. For example, we don't
> provide the ability to use it as a temporary object database source that
> allows the user to write objects, but discard them after Git exists. So
> while these cached objects behave almost like a source, they aren't used
> as one.

I find the wording of the second bullet point and paragraph above a
little confusing. Are there existing uses where new objects are written
to only the cache?

> This is about to change over the following commits, where we will turn
> cached objects into a new "inmemory" source. This will allow us to use
> it exactly the same as any other source by providing the same common
> interface as the "files" source.

Treating the object cache just like any other ODB source seems like a
good direction.

> For now, the inmemory source only hosts the cached objects and doesn't
> provide any logic yet. This will change with subsequent commits, where
> we move respective functionality into the source.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Makefile              |  1 +
>  meson.build           |  1 +
>  odb.c                 | 21 +++++++++++++--------
>  odb.h                 |  4 ++--
>  odb/source-inmemory.c | 12 ++++++++++++
>  odb/source-inmemory.h | 35 +++++++++++++++++++++++++++++++++++
>  odb/source.h          |  3 +++
>  7 files changed, 67 insertions(+), 10 deletions(-)
> 
[snip]
> @@ -1123,9 +1126,11 @@ void odb_free(struct object_database *o)
>  	odb_close(o);
>  	odb_free_sources(o);
>  
> -	for (size_t i = 0; i < o->cached_object_nr; i++)
> -		free((char *) o->cached_objects[i].value.buf);
> -	free(o->cached_objects);
> +	for (size_t i = 0; i < o->inmemory_objects->objects_nr; i++)
> +		free((char *) o->inmemory_objects->objects[i].value.buf);
> +	free(o->inmemory_objects->objects);
> +	free(o->inmemory_objects->base.path);
> +	free(o->inmemory_objects);

Should we have some sort of `odb_source_inmemory_release()`?

>  
>  	string_list_clear(&o->submodule_source_paths, 0);
>  
> diff --git a/odb.h b/odb.h
> index 3a711f6547..3d20270a05 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -8,6 +8,7 @@
>  #include "thread-utils.h"
>  
>  struct cached_object_entry;
> +struct odb_source_inmemory;
>  struct packed_git;
>  struct repository;
>  struct strbuf;
> @@ -98,8 +99,7 @@ struct object_database {
>  	 * to write them into the object store (e.g. a browse-only
>  	 * application).
>  	 */
> -	struct cached_object_entry *cached_objects;
> -	size_t cached_object_nr, cached_object_alloc;
> +	struct odb_source_inmemory *inmemory_objects;

We store an inmemory ODB source instead of the cache object info
directly. Makes sense. 

>  
>  	/*
>  	 * A fast, rough count of the number of objects in the repository.
> diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
> new file mode 100644
> index 0000000000..c7ac5c24f0
> --- /dev/null
> +++ b/odb/source-inmemory.c
> @@ -0,0 +1,12 @@
> +#include "git-compat-util.h"
> +#include "odb/source-inmemory.h"
> +
> +struct odb_source_inmemory *odb_source_inmemory_new(struct object_database *odb)
> +{
> +	struct odb_source_inmemory *source;
> +
> +	CALLOC_ARRAY(source, 1);
> +	odb_source_init(&source->base, odb, ODB_SOURCE_INMEMORY, "source", false);

huh, so we set the path for the `struct odb_source` to "source". In the
context of an inmemory source, a path doesn't make much sense. I suspect
though that storing a path is likely only useful the context of the
files ODB source. Is there reason for us to still keep this around in
the generic ODB source?

> +
> +	return source;
> +}
> diff --git a/odb/source-inmemory.h b/odb/source-inmemory.h
> new file mode 100644
> index 0000000000..95477bf36d
> --- /dev/null
> +++ b/odb/source-inmemory.h
> @@ -0,0 +1,35 @@
> +#ifndef ODB_SOURCE_INMEMORY_H
> +#define ODB_SOURCE_INMEMORY_H
> +
> +#include "odb/source.h"
> +
> +struct cached_object_entry;
> +
> +/*
> + * An inmemory source that you can write objects to that shall be made
> + * available for reading, but that shouldn't ever be persisted to disk. Note
> + * that any objects written to this source will be stored in memory, so the
> + * number of objects you can store is limited by available system memory.
> + */
> +struct odb_source_inmemory {
> +	struct odb_source base;
> +
> +	struct cached_object_entry *objects;
> +	size_t objects_nr, objects_alloc;
> +};

This new ODB source now just contains the object cache info. Looks good.

-Justin