[pacman-dev] [PATCH 1/6] Simplify hash function to a single multiplication
More than likely the compiler will do the three operation breakdown we
had here before (2 shifts + subtraction), but let the compiler do the
optimizations and make the actual operation more obvious. This actually
slightly shrinks the function binary size, likely due to instruction
reordering or something.
Signed-off-by: Dan McGee
In commit 4c5e7af32f9, we changed this code to use the regex gathered
substrings. However, we failed to correctly store the delta file name
(leaking memory), as well as freeing the temporary string used to hold
the file size string.
Signed-off-by: Dan McGee
This reduces the number of regcomp() calls when parsing delta entries in
the database from once per entry to once for the entire context handle
by storing the compiled regex data on the handle itself. Just as we do
with the cURL handle, we initialize it the first time it is needed and
free it when releasing the handle.
A few other small tweaks to the parsing function also take place,
including using the stack to store the transient and short file size
string while parsing it.
When parsing a sync database with 1378 delta entries, this reduces the
time of a `pacman -Sl deltas` operation by 50% from 0.22s to 0.12s.
Signed-off-by: Dan McGee
On 01/01/12 13:07, Dan McGee wrote:
This reduces the number of regcomp() calls when parsing delta entries in the database from once per entry to once for the entire context handle by storing the compiled regex data on the handle itself. Just as we do with the cURL handle, we initialize it the first time it is needed and free it when releasing the handle.
A few other small tweaks to the parsing function also take place, including using the stack to store the transient and short file size string while parsing it.
When parsing a sync database with 1378 delta entries, this reduces the time of a `pacman -Sl deltas` operation by 50% from 0.22s to 0.12s.
Signed-off-by: Dan McGee
--- Test database, built from my local cache that was used for above numbers: https://dev.archlinux.org/~dan/deltas.db
Should be useful for delta-related code in general.
lib/libalpm/delta.c | 33 +++++++++++++++++++-------------- lib/libalpm/delta.h | 2 +- lib/libalpm/handle.c | 3 +++ lib/libalpm/handle.h | 5 +++++ 4 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c index 165cdef..726f03c 100644 --- a/lib/libalpm/delta.c +++ b/lib/libalpm/delta.c @@ -273,29 +273,32 @@ alpm_list_t SYMEXPORT *alpm_pkg_unused_deltas(alpm_pkg_t *pkg) * This function assumes that the string is in the correct format. * This format is as follows: * $deltafile $deltamd5 $deltasize $oldfile $newfile + * @param handle the context handle * @param line the string to parse * @return A pointer to the new alpm_delta_t object */ -/* TODO this does not really belong here, but in a parsing lib */ -alpm_delta_t *_alpm_delta_parse(char *line) +alpm_delta_t *_alpm_delta_parse(alpm_handle_t *handle, const char *line)
Changed function prototype so should also change the one usage of this function in the codebase... Fixup patch on my patchqueue branch. Allan
On Sun, Jan 1, 2012 at 10:49 PM, Allan McRae
On 01/01/12 13:07, Dan McGee wrote:
This reduces the number of regcomp() calls when parsing delta entries in the database from once per entry to once for the entire context handle by storing the compiled regex data on the handle itself. Just as we do with the cURL handle, we initialize it the first time it is needed and free it when releasing the handle.
A few other small tweaks to the parsing function also take place, including using the stack to store the transient and short file size string while parsing it.
When parsing a sync database with 1378 delta entries, this reduces the time of a `pacman -Sl deltas` operation by 50% from 0.22s to 0.12s.
Signed-off-by: Dan McGee
--- Test database, built from my local cache that was used for above numbers: https://dev.archlinux.org/~dan/deltas.db
Should be useful for delta-related code in general.
lib/libalpm/delta.c | 33 +++++++++++++++++++-------------- lib/libalpm/delta.h | 2 +- lib/libalpm/handle.c | 3 +++ lib/libalpm/handle.h | 5 +++++ 4 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c index 165cdef..726f03c 100644 --- a/lib/libalpm/delta.c +++ b/lib/libalpm/delta.c @@ -273,29 +273,32 @@ alpm_list_t SYMEXPORT *alpm_pkg_unused_deltas(alpm_pkg_t *pkg) * This function assumes that the string is in the correct format. * This format is as follows: * $deltafile $deltamd5 $deltasize $oldfile $newfile + * @param handle the context handle * @param line the string to parse * @return A pointer to the new alpm_delta_t object */ -/* TODO this does not really belong here, but in a parsing lib */ -alpm_delta_t *_alpm_delta_parse(char *line) +alpm_delta_t *_alpm_delta_parse(alpm_handle_t *handle, const char *line)
Changed function prototype so should also change the one usage of this function in the codebase...
Good catch of course, my rebase-foo was not working well and this got swallowed into a different patch- thanks for actually testing these. :) -Dan
We don't need absolute floating point precision at all here; we can
stick to integer land and use milliseconds which are precise enough for
our purposes. This also removes most floating point math out of the
non-update code path.
Signed-off-by: Dan McGee
Signed-off-by: Dan McGee
On Sat, Dec 31, 2011 at 09:07:12PM -0600, Dan McGee wrote:
Signed-off-by: Dan McGee
--- If anyone has any exotic builds that this breaks in, let me know, but since we now install everything else we should probably do these too. This vastly simplifies the pacman-contrib PKGBUILD to say the least.
contrib/Makefile.am | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index 8751fd9..73494e8 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -64,6 +64,12 @@ $(OURFILES): Makefile
all-am: $(OURSCRIPTS) $(OURFILES)
+install-data-local: + $(MKDIR_P) $(DESTDIR)$(sysconfdir)/bash_completion.d/ + $(INSTALL_DATA) bash_completion $(DESTDIR)$(sysconfdir)/bash_completion.d/pacman + $(MKDIR_P) $(DESTDIR)$(datarootdir)/zsh/site-functions/ + $(INSTALL_DATA) zsh_completion $(DESTDIR)$(datarootdir)/zsh/site-functions/_pacman +
Don't we want an uninstall rule to match this? I recall having to add them when I worked on the symlink voodoo for repo-add/remove and the manpages.
bacman: $(srcdir)/bacman.in bash_completion: $(srcdir)/bash_completion.in paccache: $(srcdir)/paccache.in -- 1.7.8.1
On Sat, Dec 31, 2011 at 9:09 PM, Dave Reisner
On Sat, Dec 31, 2011 at 09:07:12PM -0600, Dan McGee wrote:
Signed-off-by: Dan McGee
--- If anyone has any exotic builds that this breaks in, let me know, but since we now install everything else we should probably do these too. This vastly simplifies the pacman-contrib PKGBUILD to say the least.
contrib/Makefile.am | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index 8751fd9..73494e8 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -64,6 +64,12 @@ $(OURFILES): Makefile
all-am: $(OURSCRIPTS) $(OURFILES)
+install-data-local: + $(MKDIR_P) $(DESTDIR)$(sysconfdir)/bash_completion.d/ + $(INSTALL_DATA) bash_completion $(DESTDIR)$(sysconfdir)/bash_completion.d/pacman + $(MKDIR_P) $(DESTDIR)$(datarootdir)/zsh/site-functions/ + $(INSTALL_DATA) zsh_completion $(DESTDIR)$(datarootdir)/zsh/site-functions/_pacman +
Don't we want an uninstall rule to match this? I recall having to add them when I worked on the symlink voodoo for repo-add/remove and the manpages.
Done, good call.
This reduces the number of functions we call by log(n) in this function,
and the inlined version is trivial and barely increases the size of the
function.
Signed-off-by: Dan McGee
participants (4)
-
Allan McRae
-
Dan McGee
-
Dan McGee
-
Dave Reisner