From 49a525f6a5b4f99a85bbe653684afd111fa92070 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 14 Apr 2017 15:15:23 -0400
Subject: [PATCH] repo: Optimize bare-user content object reads a bit
`perf record ostree checkout ...` for a bare-user repo was telling
me we were spending a good 13% of our time in the depchain of `ot_lgexattrat()`.
The problem here is that traversing the `/proc` path turns out to be
somewhat expensive - there are LSM (SELinux) checks, etc.
For regular files, opening and just getting the xattr, then closing is still
quite cheap. For symlinks, we'll always need to open anyways.
This appears to shave about ~0.1 seconds off of a checkout of
`fedora-atomic/25/x86_64/docker-host` on my workstation.
Oh, and this was the last user of `ot_lgexattrat()` so we can kill it, which is
nice - the xattr code should really live in libglnx.
Closes: #796
Approved by: jlebon
---
src/libostree/ostree-repo.c | 29 +++++++---------
src/libotutil/ot-fs-utils.c | 66 -------------------------------------
src/libotutil/ot-fs-utils.h | 13 --------
3 files changed, 12 insertions(+), 96 deletions(-)
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index c8a12543..bfb9fb37 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -2770,8 +2770,18 @@ ostree_repo_load_file (OstreeRepo *self,
g_autoptr(GBytes) bytes = NULL;
glnx_fd_close int fd = -1;
- bytes = ot_lgetxattrat (self->objects_dir_fd, loose_path_buf,
- "user.ostreemeta", error);
+ /* In bare-user, symlinks are stored as regular files, so we just
+ * always do an open, then query the user.ostreemeta xattr for
+ * more information.
+ */
+ fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
+ if (fd < 0)
+ {
+ glnx_set_error_from_errno (error);
+ goto out;
+ }
+
+ bytes = glnx_fgetxattr_bytes (fd, "user.ostreemeta", error);
if (bytes == NULL)
goto out;
@@ -2783,21 +2793,6 @@ ostree_repo_load_file (OstreeRepo *self,
mode = g_file_info_get_attribute_uint32 (ret_file_info, "unix::mode");
- /* Optimize this so that we only open the file if we
- * need to; symlinks contain their content, and we only
- * open regular files if the caller has requested an
- * input stream.
- */
- if (S_ISLNK (mode) || out_input)
- {
- fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
- if (fd < 0)
- {
- glnx_set_error_from_errno (error);
- goto out;
- }
- }
-
if (S_ISREG (mode) && out_input)
{
g_assert (fd != -1);
diff --git a/src/libotutil/ot-fs-utils.c b/src/libotutil/ot-fs-utils.c
index a0a72ca5..4ecf820b 100644
--- a/src/libotutil/ot-fs-utils.c
+++ b/src/libotutil/ot-fs-utils.c
@@ -61,72 +61,6 @@ ot_gopendirat (int dfd,
return TRUE;
}
-GBytes *
-ot_lgetxattrat (int dfd,
- const char *path,
- const char *attribute,
- GError **error)
-{
- /* A workaround for the lack of lgetxattrat(), thanks to Florian Weimer:
- * https://mail.gnome.org/archives/ostree-list/2014-February/msg00017.html
- */
- g_autofree char *full_path = g_strdup_printf ("/proc/self/fd/%d/%s", dfd, path);
- GBytes *bytes = NULL;
- ssize_t bytes_read, real_size;
- char *buf;
-
- do
- bytes_read = lgetxattr (full_path, attribute, NULL, 0);
- while (G_UNLIKELY (bytes_read < 0 && errno == EINTR));
- if (G_UNLIKELY (bytes_read < 0))
- {
- glnx_set_error_from_errno (error);
- goto out;
- }
-
- buf = g_malloc (bytes_read);
- do
- real_size = lgetxattr (full_path, attribute, buf, bytes_read);
- while (G_UNLIKELY (real_size < 0 && errno == EINTR));
- if (G_UNLIKELY (real_size < 0))
- {
- glnx_set_error_from_errno (error);
- g_free (buf);
- goto out;
- }
-
- bytes = g_bytes_new_take (buf, real_size);
- out:
- return bytes;
-}
-
-gboolean
-ot_lsetxattrat (int dfd,
- const char *path,
- const char *attribute,
- const void *value,
- gsize value_size,
- int flags,
- GError **error)
-{
- /* A workaround for the lack of lsetxattrat(), thanks to Florian Weimer:
- * https://mail.gnome.org/archives/ostree-list/2014-February/msg00017.html
- */
- g_autofree char *full_path = g_strdup_printf ("/proc/self/fd/%d/%s", dfd, path);
- int res;
-
- do
- res = lsetxattr (full_path, "user.ostreemeta", value, value_size, flags);
- while (G_UNLIKELY (res == -1 && errno == EINTR));
- if (G_UNLIKELY (res == -1))
- {
- glnx_set_error_from_errno (error);
- return FALSE;
- }
-
- return TRUE;
-}
-
gboolean
ot_readlinkat_gfile_info (int dfd,
const char *path,
diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h
index 13909c8e..edf8b29a 100644
--- a/src/libotutil/ot-fs-utils.h
+++ b/src/libotutil/ot-fs-utils.h
@@ -33,19 +33,6 @@ gboolean ot_gopendirat (int dfd,
int *out_fd,
GError **error);
-GBytes * ot_lgetxattrat (int dfd,
- const char *path,
- const char *attribute,
- GError **error);
-
-gboolean ot_lsetxattrat (int dfd,
- const char *path,
- const char *attribute,
- const void *value,
- gsize value_size,
- int flags,
- GError **error);
-
gboolean ot_readlinkat_gfile_info (int dfd,
const char *path,
GFileInfo *target_info,