State of the optimization pach
Marco Diego Aurélio Mesquita
marcodiegomesquita at gmail.com
Sun Oct 9 18:14:03 UTC 2016
Hi devs!
I'm writing to inform you about the state of the optimization patch.
I'm attaching the latest version of it so anybody can test it or make
suggestions. It still does not address all the issues reported by Dan,
but can be tested the way it is now. Matthew, if you can, please check
if is there any performance improvement in this last version.
This latest version addresses the issues of space before parenthesis,
path_position hash table is gone, declaration of variables have been
moved to the beginning of blocks. It is still missing the removal of
the add_package function, removal of a memory leak fix and whitespace
fixes. Other changes that have not been done were because they break
some test.
AFAICT, the next version of this patch will be sent through bugzilla
and last remaining issues will be fixed, so you will be able to
properly review (or integrate) it. Apart from that, is there any
advice (lint command) on how to fix the whitespace issues? I think
this is the last remaining big problem to be fixed.
Thank you!
-------------- next part --------------
diff --git a/main.c b/main.c
index 227ed35..9b27d9a 100644
--- a/main.c
+++ b/main.c
@@ -643,7 +643,7 @@ main (int argc, char **argv)
return 1;
}
- package_init ();
+ package_init (want_list);
if (want_list)
{
diff --git a/pkg.c b/pkg.c
index 5aa4380..e8dafef 100644
--- a/pkg.c
+++ b/pkg.c
@@ -43,11 +43,8 @@
static void verify_package (Package *pkg);
static GHashTable *packages = NULL;
-static GHashTable *locations = NULL;
-static GHashTable *path_positions = NULL;
static GHashTable *globals = NULL;
static GList *search_dirs = NULL;
-static int scanned_dir_count = 0;
gboolean disable_uninstalled = FALSE;
gboolean ignore_requires = FALSE;
@@ -121,7 +118,39 @@ name_ends_in_uninstalled (const char *str)
return FALSE;
}
+static char *
+add_package (const char *dirname, const char *filename)
+{
+ unsigned const int dirnamelen = strlen (dirname);
+ int len = strlen (filename);
+ char *pkgname;
+
+ if (!ends_in_dotpc (filename))
+ {
+ debug_spew ("Ignoring file '%s' in search directory; not a .pc file\n",
+ filename);
+ return NULL;
+ }
+
+ pkgname = g_malloc (len - EXT_LEN + 1);
+
+ debug_spew ("File '%s' appears to be a .pc file\n", filename);
+
+ strncpy (pkgname, filename, len - EXT_LEN);
+ pkgname[len-EXT_LEN] = '\0';
+
+ char *path = g_malloc (dirnamelen + 1 + len + 1);
+ strncpy (path, dirname, dirnamelen);
+ path[dirnamelen] = G_DIR_SEPARATOR;
+ strcpy (path + dirnamelen + 1, filename);
+
+ debug_spew ("Will find package '%s' in file '%s'\n",
+ pkgname, path);
+ return path;
+}
+static Package *
+internal_get_package (const char *name, gboolean warn);
/* Look for .pc files in the given directory and add them into
* locations, ignoring duplicates
*/
@@ -129,7 +158,7 @@ static void
scan_dir (char *dirname)
{
GDir *dir;
- const gchar *d_name;
+ const gchar *filename;
int dirnamelen = strlen (dirname);
/* Use a copy of dirname cause Win32 opendir doesn't like
@@ -161,59 +190,19 @@ scan_dir (char *dirname)
dir = g_dir_open (dirname_copy, 0 , NULL);
g_free (dirname_copy);
- scanned_dir_count += 1;
-
if (!dir)
{
- debug_spew ("Cannot open directory #%i '%s' in package search path: %s\n",
- scanned_dir_count, dirname, g_strerror (errno));
+ debug_spew ("Cannot open directory '%s' in package search path: %s\n", dirname, g_strerror (errno));
return;
}
- debug_spew ("Scanning directory #%i '%s'\n", scanned_dir_count, dirname);
-
- while ((d_name = g_dir_read_name(dir)))
- {
- int len = strlen (d_name);
-
- if (ends_in_dotpc (d_name))
- {
- char *pkgname = g_malloc (len - EXT_LEN + 1);
-
- debug_spew ("File '%s' appears to be a .pc file\n", d_name);
-
- strncpy (pkgname, d_name, len - EXT_LEN);
- pkgname[len-EXT_LEN] = '\0';
-
- if (g_hash_table_lookup (locations, pkgname))
- {
- debug_spew ("File '%s' ignored, we already know about package '%s'\n", d_name, pkgname);
- g_free (pkgname);
- }
- else
- {
- char *filename = g_malloc (dirnamelen + 1 + len + 1);
- strncpy (filename, dirname, dirnamelen);
- filename[dirnamelen] = G_DIR_SEPARATOR;
- strcpy (filename + dirnamelen + 1, d_name);
+ debug_spew ("Scanning directory '%s'\n", dirname);
- if (g_file_test(filename, G_FILE_TEST_IS_REGULAR) == TRUE) {
- g_hash_table_insert (locations, pkgname, filename);
- g_hash_table_insert (path_positions, pkgname,
- GINT_TO_POINTER (scanned_dir_count));
- debug_spew ("Will find package '%s' in file '%s'\n",
- pkgname, filename);
- } else {
- debug_spew ("Ignoring '%s' while looking for '%s'; not a "
- "regular file.\n", pkgname, filename);
- }
- }
- }
- else
+ while ((filename = g_dir_read_name(dir)))
{
- debug_spew ("Ignoring file '%s' in search directory; not a .pc file\n",
- d_name);
- }
+ char *path = g_strdup_printf ("%s/%s", dirname, filename);
+ internal_get_package (path, FALSE);
+ g_free (path);
}
g_dir_close (dir);
}
@@ -243,7 +232,7 @@ add_virtual_pkgconfig_package (void)
}
void
-package_init ()
+package_init (gboolean want_list)
{
static gboolean initted = FALSE;
@@ -252,13 +241,39 @@ package_init ()
initted = TRUE;
packages = g_hash_table_new (g_str_hash, g_str_equal);
- locations = g_hash_table_new (g_str_hash, g_str_equal);
- path_positions = g_hash_table_new (g_str_hash, g_str_equal);
+ if (want_list)
+ g_list_foreach (search_dirs, (GFunc)scan_dir, NULL);
+ else
add_virtual_pkgconfig_package ();
+ }
+}
- g_list_foreach (search_dirs, (GFunc)scan_dir, NULL);
+static char *
+search_package (const char *pkgname, unsigned int *path_position)
+{
+ char *ret = NULL;
+
+ char *filename = g_strdup_printf ("%s.pc", pkgname);
+ *path_position = 0;
+ GList *dir_iter;
+
+ for (dir_iter = search_dirs; dir_iter != NULL; dir_iter = g_list_next (dir_iter))
+ {
+ *path_position = *path_position + 1;
+ char *dirname = dir_iter->data;
+ char *path = g_strdup_printf ("%s/%s", dirname, filename);
+ const gboolean file_exists = g_file_test (path, G_FILE_TEST_IS_REGULAR);
+ free (path);
+ if (file_exists) {
+ ret = add_package (dirname, filename);
+ break;
+ }
}
+
+ g_free (filename);
+
+ return ret;
}
static Package *
@@ -267,6 +282,7 @@ internal_get_package (const char *name, gboolean warn)
Package *pkg = NULL;
char *key;
const char *location;
+ unsigned int path_position = 0;
GList *iter;
pkg = g_hash_table_lookup (packages, name);
@@ -302,8 +318,7 @@ internal_get_package (const char *name, gboolean warn)
return pkg;
}
}
-
- location = g_hash_table_lookup (locations, name);
+ location = search_package (name, &path_position);
}
if (location == NULL)
@@ -326,8 +341,13 @@ internal_get_package (const char *name, gboolean warn)
const char *end = name + (len - EXT_LEN);
const char *start = end;
- while (start != name && *start != G_DIR_SEPARATOR)
+ while (start != name) {
--start;
+ if(start[0] == G_DIR_SEPARATOR) {
+ start++;
+ break;
+ }
+ }
g_assert (end >= start);
@@ -348,8 +368,7 @@ internal_get_package (const char *name, gboolean warn)
if (strstr (location, "uninstalled.pc"))
pkg->uninstalled = TRUE;
- pkg->path_position =
- GPOINTER_TO_INT (g_hash_table_lookup (path_positions, pkg->key));
+ pkg->path_position = path_position;
debug_spew ("Path position of '%s' is %d\n",
pkg->key, pkg->path_position);
@@ -893,7 +912,7 @@ verify_package (Package *pkg)
system_dir_iter = system_dir_iter->next;
}
}
- g_list_free (system_directories);
+ g_list_free_full (system_directories, free);
while (count)
{
@@ -1193,8 +1212,8 @@ print_package_list (void)
ignore_requires = TRUE;
ignore_requires_private = TRUE;
- g_hash_table_foreach (locations, max_len_foreach, &mlen);
- g_hash_table_foreach (locations, packages_foreach, GINT_TO_POINTER (mlen + 1));
+ g_hash_table_foreach (packages, max_len_foreach, &mlen);
+ g_hash_table_foreach (packages, packages_foreach, GINT_TO_POINTER (mlen + 1));
}
void
diff --git a/pkg.h b/pkg.h
index 13cc2c6..c6732bd 100644
--- a/pkg.h
+++ b/pkg.h
@@ -98,7 +98,7 @@ char * packages_get_var (GList *pkgs,
void add_search_dir (const char *path);
void add_search_dirs (const char *path, const char *separator);
-void package_init (void);
+void package_init (gboolean want_list);
int compare_versions (const char * a, const char *b);
gboolean version_test (ComparisonType comparison,
const char *a,
More information about the pkg-config
mailing list