From c2baa6d10b18178643bb3b40447343a22165752a Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 7 Jun 2022 20:30:09 -0400
Subject: [PATCH] repo: Optimize memory use of `ostree_repo_list_objects()`

I was looking at https://github.com/ostreedev/ostree/pull/2632
and confused at the usage of
`GVariant *value = g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0));`
which looked strange - why the empty strv?

It turns out that this is a historical legacy of the time when
ostree had pack files.  And nothing actually cares about the values
of these variants; we should have an API that returns a proper set,
and not a hash.

But...since all of these things have exactly the same value, instead
of allocating lots of redundant copies on the heap, just have
them all hold a refcount on a shared value.

This cuts the heap usage from 20MB to 13MB on a test FCOS repository
build.
---
 src/libostree/ostree-repo.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 994db626..5b1aaae9 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3895,6 +3895,7 @@ ostree_repo_get_parent (OstreeRepo  *self)
 
 static gboolean
 list_loose_objects_at (OstreeRepo             *self,
+                       GVariant               *dummy_value,
                        GHashTable             *inout_objects,
                        int                     dfd,
                        const char             *prefix,
@@ -3902,7 +3903,7 @@ list_loose_objects_at (OstreeRepo             *self,
                        GCancellable           *cancellable,
                        GError                **error)
 {
-  GVariant *key, *value;
+  GVariant *key;
 
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
   gboolean exists;
@@ -3971,11 +3972,10 @@ list_loose_objects_at (OstreeRepo             *self,
         }
 
       key = ostree_object_name_serialize (buf, objtype);
-      value = g_variant_new ("(b@as)",
-                             TRUE, g_variant_new_strv (NULL, 0));
+
       /* transfer ownership */
       g_hash_table_replace (inout_objects, g_variant_ref_sink (key),
-                            g_variant_ref_sink (value));
+                            g_variant_ref (dummy_value));
     }
 
   return TRUE;
@@ -3989,6 +3989,9 @@ list_loose_objects (OstreeRepo                     *self,
                     GError                        **error)
 {
   static const gchar hexchars[] = "0123456789abcdef";
+  // For unfortunate historical reasons we emit this dummy value.
+  g_autoptr(GVariant) dummy_loose_object_variant =
+    g_variant_ref_sink (g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0)));
 
   for (guint c = 0; c < 256; c++)
     {
@@ -3996,7 +3999,8 @@ list_loose_objects (OstreeRepo                     *self,
       buf[0] = hexchars[c >> 4];
       buf[1] = hexchars[c & 0xF];
       buf[2] = '\0';
-      if (!list_loose_objects_at (self, inout_objects, self->objects_dir_fd, buf,
+      if (!list_loose_objects_at (self, dummy_loose_object_variant,
+                                  inout_objects, self->objects_dir_fd, buf,
                                   commit_starting_with,
                                   cancellable, error))
         return FALSE;