[prev in list] [next in list] [prev in thread] [next in thread] 

List:       glibc-alpha
Subject:    Re: [PATCH] elf: Sort only uninitialized objects in _dl_map_object_deps()
From:       Xiaoming Ni <nixiaoming () huawei ! com>
Date:       2020-07-29 13:56:07
Message-ID: 0e390002-1363-1c4a-fd55-cb5f12affd71 () huawei ! com
[Download RAW message or body]

On 2020/7/27 8:36, Carlos O'Donell wrote:
> On 7/26/20 6:41 AM, Chung-Lin Tang wrote:
> > 
> > 
> > On 2020/7/26 4:57 AM, Carlos O'Donell via Libc-alpha wrote:
> > > > Run the dlopen() for each dynamic library.
> > > > Before the patch is installed, it takes 214 seconds.
> > > > After patching, it takes 37 seconds.
> > > Is it still correct? >>>
> > > Do you have a test case you can add?

I do not have fully automated test cases locally.
Perform the following steps to manually verify the configuration:
1. A large number of SOs are automatically generated. The init and exit 
functions of an SO record logs.
2. Design different dependencies (unidirectional linked list, binary 
tree, n-ary tree, ring, network, and random dependency).
3. Re-link the SO based on the dependency relationship.
4. Generate a random array and call dlopen in the sequence recorded in 
the array.
5. Manually check whether test logs and dependencies comply with the ELF 
standard.

After manual verification, the init/exit sequence of the SO still 
complies with the ELF standard.

> > > 
> > > > Signed-off-by: Xiaoming Ni<nixiaoming@huawei.com>
> > > Reviewing this will have to wait until after the release, but this patch is
> > > interesting.
> > 
> > This patch appears to add a linear pass to somewhat reduce the input size
> > of a circularly linked case, but frankly speaking, is only useful with the \
> > current old sorting algorithm, and just to a certain degree.
> 
> Chung-Lin, thank you for looking over the suggested fixes.
> 

elf/dl-deps.c  _dl_map_object_deps:

-  _dl_sort_maps (&l_initfini[1], nlist - 1, NULL, false);
+  if (map->l_init_called == 0)
+    _dl_sort_maps (&l_initfini[1], nlist - 1, NULL, false);

When an object has been loaded, all its dependent objects have been 
initialized. In this case, sorting is not required.


> > The mentioned test case still takes 37 seconds with the proposed patch, while for
> > the new DFS-based algorithm, even without any such special case input reduction,
> > the sort time will probably be instantaneous.
> 
> I expected that might be the case, but it would still be good to put
> the example into a test case and verify.
> 
> > > Have you looked at Chung-Ling Tang's most recent work in this area?
> > > 
> > > https://patchwork.sourceware.org/project/glibc/patch/1427b370-7400-afd0-16e8-55c1072db20e@mentor.com/
> > >  https://patchwork.sourceware.org/project/glibc/patch/5de3ab61-3dca-b400-15c6-92ff5ae80877@mentor.com/
> > > 
> > 
I didn't notice this before.

> > If you're trying out the #17645 sorting patch, remember to add \
> > GLIBC_TUNABLES=glibc.rtld.dynamic_sort=2 to the environment before running the \
> > test, or it will still be the old algorithm. 
> > > Could you use Chung-Ling's test case constructor to write a test case?
> > > 
> > 
> > Yeah, a new test case like this is always nice, especially to test if the \
> > description language is expressive enough to handle this.
> 
> Agreed.
> 
I'm trying to automate my test cases, but it's going to take a while.

thanks


[prev in list] [next in list] [prev in thread] [next in thread]