This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[PATCH] dl-lookup.c space saving


Hi!

do_lookup and do_lookup_versioned are quite big functions marked with static
inline and each of them is inlined 6 times, although IMHO only one place is
worth inlining and the remaining 5 places are better not inlined.
I think STV_PROTECTED symbols are rare, similarly RTLD_NEXT is not done too
often. The space savings are quite big.
This patch will work until gcc starts inlining on trees in C, after that
at -O3 it will either have to mark _dl_do_lookup* as __attribute__((dontinline))
if such thing will exist, or move them into a separate file.
Also, I found two leftover defines from initial H.J.'s STV_PROTECTED patch.

2001-05-18  Jakub Jelinek  <jakub@redhat.com>

	* dl-lookup.c (PROTECTED): Remove defines.
	(add_dependency): Mark it with internal_function.
	(_dl_do_lookup, _dl_do_lookup_versioned): New functions.
	(_dl_lookup_symbol, _dl_lookup_symbol_skip,
	_dl_lookup_versioned_symbol, _dl_lookup_versioned_symbol_skip): Use
	it if we don't want do_lookup* inlined.

--- libc/dl-lookup.c.jj	Fri May 18 09:43:34 2001
+++ libc/dl-lookup.c	Fri May 18 16:14:37 2001
@@ -75,16 +75,15 @@ __libc_lock_define (extern, _dl_load_loc
    without versioning.  gcc is not able to optimize a single function
    definition serving for both purposes so we define two functions.  */
 #define VERSIONED	0
-#define PROTECTED	0
 #include "do-lookup.h"
 
 #define VERSIONED	1
-#define PROTECTED	0
 #include "do-lookup.h"
 
 
 /* Add extra dependency on MAP to UNDEF_MAP.  */
 static int
+internal_function
 add_dependency (struct link_map *undef_map, struct link_map *map)
 {
   struct link_map **list;
@@ -180,6 +179,19 @@ add_dependency (struct link_map *undef_m
   return result;
 }
 
+static int
+internal_function
+_dl_do_lookup (const char *undef_name, unsigned long int hash,
+	       const ElfW(Sym) *ref, struct sym_val *result,
+	       struct r_scope_elem *scope, size_t i,
+	       struct link_map *skip, int noexec, int noplt);
+static int
+internal_function
+_dl_do_lookup_versioned (const char *undef_name, unsigned long int hash,
+			 const ElfW(Sym) *ref, struct sym_val *result,
+			 struct r_scope_elem *scope, size_t i,
+			 const struct r_found_version *const version,
+			 struct link_map *skip, int noexec, int noplt);
 
 /* Search loaded objects' symbol tables for a definition of the symbol
    UNDEF_NAME.  */
@@ -261,8 +273,8 @@ _dl_lookup_symbol (const char *undef_nam
       struct sym_val protected_value = { NULL, NULL };
 
       for (scope = symbol_scope; *scope; ++scope)
-	if (do_lookup (undef_name, hash, *ref, &protected_value, *scope, 0,
-		       NULL, 0, 1))
+	if (_dl_do_lookup (undef_name, hash, *ref, &protected_value, *scope,
+			   0, NULL, 0, 1))
 	  break;
 
       if (protected_value.s == NULL || protected_value.m == undef_map)
@@ -302,11 +314,11 @@ _dl_lookup_symbol_skip (const char *unde
   for (i = 0; (*scope)->r_list[i] != skip_map; ++i)
     assert (i < (*scope)->r_nlist);
 
-  if (! do_lookup (undef_name, hash, *ref, &current_value, *scope, i,
-		   skip_map, 0, 0))
+  if (! _dl_do_lookup (undef_name, hash, *ref, &current_value, *scope, i,
+		       skip_map, 0, 0))
     while (*++scope)
-      if (do_lookup (undef_name, hash, *ref, &current_value, *scope, 0,
-		     skip_map, 0, 0))
+      if (_dl_do_lookup (undef_name, hash, *ref, &current_value, *scope, 0,
+			 skip_map, 0, 0))
 	break;
 
   if (__builtin_expect (current_value.s == NULL, 0))
@@ -337,11 +349,11 @@ _dl_lookup_symbol_skip (const char *unde
       struct sym_val protected_value = { NULL, NULL };
 
       if (i >= (*scope)->r_nlist
-	  || !do_lookup (undef_name, hash, *ref, &protected_value, *scope, i,
-			 skip_map, 0, 1))
+	  || !_dl_do_lookup (undef_name, hash, *ref, &protected_value, *scope,
+			     i, skip_map, 0, 1))
 	while (*++scope)
-	  if (do_lookup (undef_name, hash, *ref, &protected_value, *scope, 0,
-			 skip_map, 0, 1))
+	  if (_dl_do_lookup (undef_name, hash, *ref, &protected_value, *scope,
+			     0, skip_map, 0, 1))
 	    break;
 
       if (protected_value.s == NULL || protected_value.m == undef_map)
@@ -464,8 +476,8 @@ _dl_lookup_versioned_symbol (const char 
       struct sym_val protected_value = { NULL, NULL };
 
       for (scope = symbol_scope; *scope; ++scope)
-	if (do_lookup_versioned (undef_name, hash, *ref, &protected_value,
-				 *scope, 0, version, NULL, 0, 1))
+	if (_dl_do_lookup_versioned (undef_name, hash, *ref, &protected_value,
+				     *scope, 0, version, NULL, 0, 1))
 	  break;
 
       if (protected_value.s == NULL || protected_value.m == undef_map)
@@ -504,11 +516,11 @@ _dl_lookup_versioned_symbol_skip (const 
   for (i = 0; (*scope)->r_list[i] != skip_map; ++i)
     assert (i < (*scope)->r_nlist);
 
-  if (! do_lookup_versioned (undef_name, hash, *ref, &current_value,
-			     *scope, i, version, skip_map, 0, 0))
+  if (! _dl_do_lookup_versioned (undef_name, hash, *ref, &current_value,
+				 *scope, i, version, skip_map, 0, 0))
     while (*++scope)
-      if (do_lookup_versioned (undef_name, hash, *ref, &current_value, *scope,
-			       0, version, skip_map, 0, 0))
+      if (_dl_do_lookup_versioned (undef_name, hash, *ref, &current_value,
+				   *scope, 0, version, skip_map, 0, 0))
 	break;
 
   if (__builtin_expect (current_value.s == NULL, 0))
@@ -552,11 +564,13 @@ _dl_lookup_versioned_symbol_skip (const 
       struct sym_val protected_value = { NULL, NULL };
 
       if (i >= (*scope)->r_nlist
-	  || !do_lookup_versioned (undef_name, hash, *ref, &protected_value,
-				   *scope, i, version, skip_map, 0, 1))
+	  || !_dl_do_lookup_versioned (undef_name, hash, *ref,
+				       &protected_value, *scope, i, version,
+				       skip_map, 0, 1))
 	while (*++scope)
-	  if (do_lookup_versioned (undef_name, hash, *ref, &protected_value,
-				   *scope, 0, version, skip_map, 0, 1))
+	  if (_dl_do_lookup_versioned (undef_name, hash, *ref,
+				       &protected_value, *scope, 0, version,
+				       skip_map, 0, 1))
 	    break;
 
       if (protected_value.s == NULL || protected_value.m == undef_map)
@@ -588,4 +602,29 @@ _dl_setup_hash (struct link_map *map)
   map->l_buckets = hash;
   hash += map->l_nbuckets;
   map->l_chain = hash;
+}
+
+/* These are here so that we only inline do_lookup{,_versioned} in the common
+   case, not everywhere.  */
+static int
+internal_function
+_dl_do_lookup (const char *undef_name, unsigned long int hash,
+	       const ElfW(Sym) *ref, struct sym_val *result,
+	       struct r_scope_elem *scope, size_t i,
+	       struct link_map *skip, int noexec, int noplt)
+{
+  return do_lookup (undef_name, hash, ref, result, scope, i, skip, noexec,
+  		    noplt);
+}
+
+static int
+internal_function
+_dl_do_lookup_versioned (const char *undef_name, unsigned long int hash,
+			 const ElfW(Sym) *ref, struct sym_val *result,
+			 struct r_scope_elem *scope, size_t i,
+			 const struct r_found_version *const version,
+			 struct link_map *skip, int noexec, int noplt)
+{
+  return do_lookup_versioned (undef_name, hash, ref, result, scope, i,
+			      version, skip, noexec, noplt);
 }

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]