bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gnulib-tool.py: Optimize --extract-recursive-dependencies.


From: Bruno Haible
Subject: Re: gnulib-tool.py: Optimize --extract-recursive-dependencies.
Date: Thu, 05 Dec 2024 09:57:21 +0100

Hi Collin,

> I believe the slowness in --extract-dependents caused by my previous
> patch was caused by the parsing of all files in modules/* done during
> GLModules.__init__().

Yes.

> Your implementation was faster for the non-recursive case, so I've added
> it back with very minor changes and used an optional top_level argument.

Thanks.

Reviewing the new code now, I see a couple of details that can be improved:

* The top_level argument is not needed. When I had suggested it, I had not
  seen that you already had split the function into two functions
  (_getDependents and getDependents). Patch 0001 eliminates it.

* getDependents is storing its result in a cache, but the cache is never read.
  This makes no sense. Either cache or not cache. I choose to cache it (just
  in case main.py needs to make the same invocation twice). Done in patch 0001
  as well.

* _getAllModules does not use the 'self' argument. This indicates that the
  method is on the wrong class. It's better done as a method on GLModuleSystem.
  Done in patch 0002.

* _getAllModules does not need to eliminate empty module names. They don't
  occur; I verified that. Also done in patch 0002.

* _getDependents has an incomplete documentation. Just saying
  "Internal function for getDependentsRecursively" is redundant.
  Fixed in patch 0003.

* Since the result of _getDependents depends on the modules list passed as
  argument, it is wrong to cache the result in self.cache[...]. Only functions
  without additional arguments are allowed to cache their values.
  Fortunately this caching is not needed, since the loop in
  getDependentsRecursively makes sure to not make repeated calls.
  Also done in patch 0003.

* The optional argument to _getDependents is overkill; it can be made a
  required argument. Also done in patch 0003.

Bruno

Attachment: 0001-gnulib-tool.py-Remove-dead-code.patch
Description: Text Data

Attachment: 0002-gnulib-tool.py-Refactor.patch
Description: Text Data

Attachment: 0003-gnulib-tool.py-Fix-invalid-use-of-cache.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]