This patch changes a variety of small things related to our pkghash
implementation with an eye toward performance, especially on native
32-bit systems.
* Use `unsigned int` rather than `size_t` for hash sizes. We already
return ERANGE for any attempted creation of a hash greater than 1
million elements, so unsigned int is more than large enough for our
purposes. Switching to this type allows 32 bit systems to do native
math without helper functions from libgcc.
* _alpm_pkghash_create() now internally adds extra padding for
additional array elements, rather than that being the responsibility of
the caller.
* #define values are moved into static const values in pkghash.c; a new
`stride` value is also extracted (but remains set at 1).
* Division and modulus operators are removed from the normal find and
add paths if possible. We store the upper limit of the number of
elements in the hash so we no longer need to calculate this every
element addition. When doing wraparound position calculations, we only
apply the modulus operator if the value is greater than the number of
buckets.
Signed-off-by: Dan McGee <dan@archlinux.org>
Ensures that config.h is always ordered correctly (first) in the
includes. Also means that new source files get this for free without
having to remember to add it.
We opt for -imacros over -include as its more portable, and the
added constraint by -imacros doesn't bother us for config.h.
This also touches the HACKING file to remove the explicit mention of
config.h as part of the includes.
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
Signed-off-by: Dan McGee <dan@archlinux.org>
Since we know the length of the line, we can use this all the way
through and do a cheaper operation than strdup() by just invoking malloc
and memcpy directly.
Signed-off-by: Dan McGee <dan@archlinux.org>
This will always be a 64-bit signed integer rather than the variable length
time_t type. Dates beyond 2038 should be fully supported in the library; the
frontend still lags behind because 32-bit platforms provide no localtime64()
or equivalent function to convert from an epoch value to a broken down time
structure.
Signed-off-by: Dan McGee <dan@archlinux.org>
Expose the current static get_pkgpath() function internally to the rest
of the library as _alpm_local_db_pkgpath(). This allows use of this
convenience function in add.c and remove.c when forming the path to the
scriptlet location.
Signed-off-by: Dan McGee <dan@archlinux.org>
We did a lot of both malloc-ing and stack printing to form some paths in
this code. Attempt to unify it all into the one get_pkgpath() method by
adding an optional third "filename" parameter, and form the necessary
path string all in one go.
Signed-off-by: Dan McGee <dan@archlinux.org>
We checked the (fgets == NULL and !feof) case, but never actually bailed
out of the loop if we were at the end of the file, causing infinite
looping.
Signed-off-by: Dan McGee <dan@archlinux.org>
This shouldn't really be declared with const, and causes a compile error
when -Wcast-qual is used. Remove the const specifier from the function
specification and all implementations.
Also fix one other trivial -Wcast-qual warning in _alpm_db_cmp().
Signed-off-by: Dan McGee <dan@archlinux.org>
Neither deltas nor filename attributes are ever present in the local
database, so we can remove all of the indirection for accessing these
attributes.
Signed-off-by: Dan McGee <dan@archlinux.org>
We currently have csize, isize, and size concepts, and sometimes the
difference isn't clear. Ensure the following holds:
* size (aka csize): always the compressed size of the package; available
for everything except local packages (where it will return 0)
* isize: always the installed size of the package; available for all
three package types
Signed-off-by: Dan McGee <dan@archlinux.org>
We were using atol(), which on 32 bit, cannot handle values greater than
2GiB, which is fail.
Switch to a strtoull() wrapper function tailored toward parsing off_t
values. This allows parsing of very large positive integer values. off_t
is a signed type, but in our usages, we never parse or have a need for
negative values, so the function will return -1 on error.
Before:
$ pacman -Si flightgear-data | grep Size
Download Size : 2097152.00 K
Installed Size : 2097152.00 K
After:
$ ./src/pacman/pacman -Si flightgear-data | grep Size
Download Size : 2312592.52 KiB
Installed Size : 5402896.00 KiB
Signed-off-by: Dan McGee <dan@archlinux.org>
Hard to believe there was still more room to improve on this, but I
found an easily correctable oversight tonight. Our databases (both sync
and local) contain many blank lines, and we were not moving onto the
next line right away in these cases; instead we would proceed through
our strcmp() conditional checks as normal.
Some local numbers follow to show the effects of this patch:
Sync `-Ss foobarbaz`:
71,709 blank lines skipped early
~1,505,889 strcmp() calls avoided (21 per line)
~15% speed improvement (.210 --> .179 sec)
Local `-Qs foobarbaz`:
6,823 blank lines skipped early
115,991 strcmp() calls avoided (17 per line)
~6% speed improvement (.080 -> .071 sec)
Signed-off-by: Dan McGee <dan@archlinux.org>
Do all the checks at once; this also avoids the 'return' call after we
have allocated memory for "pkgpath" as well as tweaked the umask.
Signed-off-by: Dan McGee <dan@archlinux.org>
The functions alpm_db_get_name(), alpm_pkg_get_name(), and
alpm_pkg_get_version() are not necessary at all, so remove the calling
and indirection when used in the backend, which makes things slightly
more efficient and reduces code size.
Signed-off-by: Dan McGee <dan@archlinux.org>
* Move is_local standalone field to status enum
* Create VALID/INVALID flag pair
* Create EXISTS/MISSING flag pair
With these additional fields, we can be more intelligent with database
loading and messages to the user. We now only warn once if a sync
database does not exist and do not continue to try to load it once we
have marked it as missing.
The reason for the flags existing in pairs is so the unknown case can be
represented. There should never be a time when both flags in the same
group are true, but if they are both false, it represents the unknown
case. Care is taken to always manipulate both flags at the same time.
Signed-off-by: Dan McGee <dan@archlinux.org>
We did this with depends way back in commit c244cfecf6 in 2007. We
can do it with these fields as well.
Of note is the inclusion of provides even though only '=' is supported-
we'll parse other things, but no guarantees are given as to behavior,
which is more or less similar to before since we only looked for the
equals sign.
Also of note is the non-inclusion of optdepends; this will likely be
resolved down the road.
The biggest benefactors of this change will be the resolving code that
formerly had to parse and reparse several of these fields; it only
happens once now at load time. This does lead to the disadvantage that
we will now always be parsing this information up front even if we never
need it in the split form, but as these are uncommon fields and our
parser is quite efficient it shouldn't be a big concern.
Signed-off-by: Dan McGee <dan@archlinux.org>
These items are never present in anything but sync databases, nor do we
even try to load them from the local database. Remvoe the indirection
meant to allow the caching layer to work since it will never do anything
anyway.
Signed-off-by: Dan McGee <dan@archlinux.org>
This adds a field in the package struct for this checksum type as well
as allowing access via the API to it. The frontend is now able to
display any read value. Note that this does not implement any use or
verification of the value internally.
Signed-off-by: Dan McGee <dan@archlinux.org>
If we are missing a local database file, we get repeated messages over
and over telling us the same thing, rather than being sane and erroring
only once. This package adds an INFRQ_ERROR level that is added to the
mask if we encounter any errors on a local_db_read() operation, and
short circuits future calls if found in the value. This fixes FS#25313.
Note that this does not make any behavior changes other than suppressing
error messages and repeated code calls to failure cases; we still have
more to do in the "local database is hosed" department.
Also make a small update to the wrong but unused flags set in
be_package; using INFRQ_ALL there was not totally correct.
Signed-off-by: Dan McGee <dan@archlinux.org>
We don't write with extra or unknown whitespace, so there is little
reason for us to trim it when reading either. This also fixes the
hopefully never encountered "paths that start or end with spaces" issue,
for which two pactests have been added. The tests also contain other
evil characters that we have encountered before and handle just fine,
but it doesn't hurt to ensure we don't break such support in the future.
Signed-off-by: Dan McGee <dan@archlinux.org>
This accomplishes quite a few things with one rather invasive change.
1. Iteration is much more performant, due to a reduction in pointer
chasing and linear item access.
2. Data structures are smaller- we no longer have the overhead of the
linked list as the file struts are now laid out consecutively in
memory.
3. Memory allocation has been massively reworked. Before, we would
allocate three different pieces of memory per file item- the list
struct, the file struct, and the copied filename. What this resulted
in was massive fragmentation of memory when loading filelists since
the memory allocator had to leave holes all over the place. The new
situation here now removes the need for any list item allocation;
allocates the file structs in contiguous memory (and reallocs as
necessary), leaving only the strings as individually allocated. Tests
using valgrind (massif) show some pretty significant memory
reductions on the worst case `pacman -Ql > /dev/null` (366387 files
on my machine):
Before:
Peak heap: 54,416,024 B
Useful heap: 36,840,692 B
Extra heap: 17,575,332 B
After:
Peak heap: 38,004,352 B
Useful heap: 28,101,347 B
Extra heap: 9,903,005 B
Several small helper methods have been introduced, including a list to
array conversion helper as well as a filelist merge sort that works
directly on arrays.
Signed-off-by: Dan McGee <dan@archlinux.org>
This allows us to capture size and mode data when building filelists
from package files. Future patches will take advantage of this newly
available information, and frontends can use it as well.
Signed-off-by: Dan McGee <dan@archlinux.org>
There is little need to expose the guts of this function even within the
library. Make it static in be_local.c, and clean up a few other things
since we know exactly where it is being called from:
* Remove unnecessary origin checks in _cache_get_*() methods- if you are
calling a cache method your package type will be correct.
* Remove sanity checks within local_db_read() itself- packages will
always have a name and version if they get this far, and the package
object will never be NULL either.
The one case calling this from outside the backend was in add.c, where
we forced a full load of a package before we duplicated it. Move this
concern elsewhere and have pkg_dup() always force a full package load
via a new force_load() function on the operations callback struct.
Signed-off-by: Dan McGee <dan@archlinux.org>
We can reorganize things a bit to not require reading a directory-only
entry first (or at all). This was noticed while working on some pactest
improvements, but should be a good step forward anyway.
Also make _alpm_splitname() a bit more generic in where it stores the
data it parses.
Signed-off-by: Dan McGee <dan@archlinux.org>
Start by converting all of our flags to a 'status' bitmask (pkgcache
status, grpcache status). Add a new 'valid' flag as well. This will let
us keep track if the database itself has been marked valid in whatever
fashion.
For local databases at the moment we ensure there are no depends files;
for sync databases we ensure the PGP signature is valid if
required/requested. The loading of the pkgcache is prohibited if the
database is invalid.
Signed-off-by: Dan McGee <dan@archlinux.org>
This is another step toward doing both local database validation
(ensuring we don't have depends files) and sync database validation (via
signatures if present) when the database is registered.
Signed-off-by: Dan McGee <dan@archlinux.org>
This allows us to separate the name and hash elements in one place and
not scatter different parsing code all over the place, including both
the frontend and backend.
Signed-off-by: Dan McGee <dan@archlinux.org>
Added a line to the top of each of be_local.c, be_package.c, and
be_sync.c indicating their purposes.
Signed-off-by: Kerrick Staley <mail@kerrickstaley.com>
Signed-off-by: Dan McGee <dan@archlinux.org>
This is the last user of our global handle object. Once again the diff
is large but the functional changes are not.
Signed-off-by: Dan McGee <dan@archlinux.org>
This requires a lot of line changes, but not many functional changes as
more often than not our handle variable is already available in some
fashion.
Signed-off-by: Dan McGee <dan@archlinux.org>
These new method signatures return and take handle objects to operate on
so we can move away from the idea of one global handle in the API. There
is also another important change and that deals with the setting of root
and dbpaths. These are now done at initialization time instead of using
setter methods. This allows the library to operate more safely knowing
that paths won't change underneath it.
Signed-off-by: Dan McGee <dan@archlinux.org>
This will make the patching process less invasive as we start to remove
this variable from all source files.
Signed-off-by: Dan McGee <dan@archlinux.org>
Similar to what we just did for the database; this will make it easy to
always know what handle a given package originated from.
Signed-off-by: Dan McGee <dan@archlinux.org>
This is the first step in a long process to remove our dependence on the
global handle variable we currently share in libalpm, with the goal to
make things a bit more thread-safe and re-entrant.
Signed-off-by: Dan McGee <dan@archlinux.org>
