On 27/04/19(Sat) 21:45, Nathanael Rensen wrote:
> The diff below speeds up ld.so library loading where the dependency tree
> is broad and deep, such as samba's smbd which links over 100 libraries.
>
> See for example https://marc.info/?l=openbsd-misc&m=155007285712913&w=2
>
> The timings below are for ldd /usr/local/sbin/smbd:
>
> Timing without diff: 2m02.50s real 2m02.48s user 0m00.02s system
> Timing with diff : 0m00.03s real 0m00.02s user 0m00.01s system
>
> Note that these timings are for a build of a recent samba master tree
> (linked with kerberos) which is probably slower than the OpenBSD port.
>
> While this diff speeds up loading (e.g. ldd) a second diff (part 2) is
> also helpful to speed up library initialisation.
As for the other diff, could you explain in words what does your change
do?
Comments below :)
> Index: libexec/ld.so/dlfcn.c
> ===================================================================
> RCS file: /cvs/src/libexec/ld.so/dlfcn.c,v
> retrieving revision 1.102
> diff -u -p -p -u -r1.102 dlfcn.c
> --- libexec/ld.so/dlfcn.c 22 Oct 2018 01:59:08 -0000 1.102
> +++ libexec/ld.so/dlfcn.c 27 Apr 2019 13:24:02 -0000
> @@ -93,7 +93,7 @@ dlopen(const char *libname, int flags)
> /* if opened but grpsym_list has not been created */
> if (OBJECT_DLREF_CNT(object) == 1) {
> /* add first object manually */
> - _dl_link_grpsym(object, 1);
> + _dl_link_grpsym(object);
> _dl_cache_grpsym_list(object);
> }
> goto loaded;
> Index: libexec/ld.so/library_subr.c
> ===================================================================
> RCS file: /cvs/src/libexec/ld.so/library_subr.c,v
> retrieving revision 1.48
> diff -u -p -p -u -r1.48 library_subr.c
> --- libexec/ld.so/library_subr.c 28 Aug 2017 14:06:22 -0000 1.48
> +++ libexec/ld.so/library_subr.c 27 Apr 2019 13:24:02 -0000
> @@ -527,19 +527,13 @@ _dl_link_child(elf_object_t *dep, elf_ob
> static unsigned int _dl_grpsym_gen = 0;
>
> void
> -_dl_link_grpsym(elf_object_t *object, int checklist)
> +_dl_link_grpsym(elf_object_t *object)
> {
> struct dep_node *n;
>
> - if (checklist) {
> - TAILQ_FOREACH(n, &_dl_loading_object->grpsym_list, next_sib)
> - if (n->data == object)
> - return; /* found, dont bother adding */
> - } else {
> - if (object->grpsym_gen == _dl_grpsym_gen) {
> + TAILQ_FOREACH(n, &_dl_loading_object->grpsym_list, next_sib)
> + if (n->data == object)
> return; /* found, dont bother adding */
> - }
> - }
Currently this loop isn't executed when being called from
_dl_cache_grpsym_list(). Is there any risk of adding a duplicate? Does
it buy us much to remove the lookup?
> object->grpsym_gen = _dl_grpsym_gen;
>
> n = _dl_malloc(sizeof *n);
> @@ -572,6 +566,7 @@ _dl_cache_grpsym_list_setup(elf_object_t
> void
> _dl_cache_grpsym_list(elf_object_t *object)
> {
> + int recurse = 0;
> struct dep_node *n;
>
> /*
> @@ -581,8 +576,12 @@ _dl_cache_grpsym_list(elf_object_t *obje
> */
>
> TAILQ_FOREACH(n, &object->child_list, next_sib)
> - _dl_link_grpsym(n->data, 0);
> + if (n->data->grpsym_gen != _dl_grpsym_gen) {
> + _dl_link_grpsym(n->data);
> + recurse = 1;
> + }
Do I understand correctly that you're not recursing if not object is
inserted? Would it makes sense to have _dl_link_grpsym() return a
value reflecting if it inserted an object or not?
>
> - TAILQ_FOREACH(n, &object->child_list, next_sib)
> - _dl_cache_grpsym_list(n->data);
> + if (recurse)
> + TAILQ_FOREACH(n, &object->child_list, next_sib)
> + _dl_cache_grpsym_list(n->data);
> }
> Index: libexec/ld.so/loader.c
> ===================================================================
> RCS file: /cvs/src/libexec/ld.so/loader.c,v
> retrieving revision 1.177
> diff -u -p -p -u -r1.177 loader.c
> --- libexec/ld.so/loader.c 3 Dec 2018 05:29:56 -0000 1.177
> +++ libexec/ld.so/loader.c 27 Apr 2019 13:24:02 -0000
> @@ -360,7 +360,7 @@ _dl_load_dep_libs(elf_object_t *object,
> }
>
> /* add first object manually */
> - _dl_link_grpsym(object, 1);
> + _dl_link_grpsym(object);
> _dl_cache_grpsym_list_setup(object);
>
> return(0);
> @@ -543,7 +543,7 @@ _dl_boot(const char **argv, char **envp,
> _dl_add_object(dyn_obj);
>
> dyn_obj->refcount++;
> - _dl_link_grpsym(dyn_obj, 1);
> + _dl_link_grpsym(dyn_obj);
>
> dyn_obj->status |= STAT_RELOC_DONE;
> _dl_set_sod(dyn_obj->load_name, &dyn_obj->sod);
> Index: libexec/ld.so/resolve.h
> ===================================================================
> RCS file: /cvs/src/libexec/ld.so/resolve.h,v
> retrieving revision 1.90
> @@ -271,7 +272,7 @@ int _dl_load_dep_libs(elf_object_t *obje
> int _dl_rtld(elf_object_t *object);
> void _dl_call_init(elf_object_t *object);
> void _dl_link_child(elf_object_t *dep, elf_object_t *p);
> -void _dl_link_grpsym(elf_object_t *object, int checklist);
> +void _dl_link_grpsym(elf_object_t *object);
> void _dl_cache_grpsym_list(elf_object_t *object);
> void _dl_cache_grpsym_list_setup(elf_object_t *object);
> void _dl_link_grpref(elf_object_t *load_group, elf_object_t *load_object);
>
