Commit 2ba7020f by Patrick Steinhardt

config_file: avoid re-reading files on write

When we rewrite the configuration file due to any of its values
being modified, we call `config_refresh` to update the in-memory
representation of our config file backend. This is needlessly
wasteful though, as `config_refresh` will always open the on-disk
representation to reads the file contents while we already know
the complete file contents at this point in time as we have just
written it to disk.

Implement a new function `config_refresh_from_buffer` that will
refresh the backend's config entries from a buffer instead of
from the config file itself. Note that this will thus _not_
update the backend's timestamp, which will cause us to re-read
the buffer when performing a read operation on it. But this is
still an improvement as we now lazily re-read the contents, and
most importantly we will avoid constantly re-reading the contents
if we perform multiple write operations.

The following strace demonstrates this if we're re-writing a key
multiple times. It uses our config example with `config_set`
changed to update the file 10 times with different keys:

	$ strace lg2 config x.x z |& grep '^open.*config'
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/home/pks/.config/git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3

And now with the optimization of `config_refresh_from_buffer`:

	$ strace lg2 config x.x z |& grep '^open.*config'
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/home/pks/.config/git/config", O_RDONLY|O_CLOEXEC) = 3
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4
	open("/tmp/repo/.git/config.lock", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 3
	open("/tmp/repo/.git/config", O_RDONLY|O_CLOEXEC) = 4

As can be seen, this is quite a lot of `open` calls less.
parent a0dc3027
......@@ -200,9 +200,27 @@ out:
return error;
}
static int config_refresh_from_buffer(git_config_backend *cfg, const char *buf, size_t buflen)
{
diskfile_backend *b = GIT_CONTAINER_OF(cfg, diskfile_backend, header.parent);
git_config_entries *entries = NULL;
int error;
if ((error = git_config_entries_new(&entries)) < 0 ||
(error = config_read_buffer(entries, b->header.repo, &b->file,
b->header.level, 0, buf, buflen)) < 0 ||
(error = config_set_entries(cfg, entries)) < 0)
goto out;
entries = NULL;
out:
git_config_entries_free(entries);
return error;
}
static int config_refresh(git_config_backend *cfg)
{
diskfile_backend *b = (diskfile_backend *)cfg;
diskfile_backend *b = GIT_CONTAINER_OF(cfg, diskfile_backend, header.parent);
git_config_entries *entries = NULL;
int error, modified;
......@@ -1230,7 +1248,7 @@ static int config_write(diskfile_backend *cfg, const char *orig_key, const char
if ((result = git_filebuf_commit(&file)) < 0)
goto done;
if ((result = config_refresh(&cfg->header.parent)) < 0)
if ((result = config_refresh_from_buffer(&cfg->header.parent, buf.ptr, buf.size)) < 0)
goto done;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment