[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Martin Dorey]

Follow-up Comment #9, bug#63650 (group make):

Interesting idea, one that doesn't seem to be ruled out by anything in the
original problem report, bug #10593.  (I have a copy of the Makefile from that
bug with an mtime one day earlier than the submission date.  It looks like my
idiolect.  Guess I accidentally raised it anonymously.)


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Raul E Rangel]

Follow-up Comment #8, bug#63650 (group make):

I just ran into this bug while trying to upgrade from 4.2 to 4.4. We had a lot
of builds get slower. Specifically
[trace-cmd](https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tag/?h=trace-cmd-v3.1.5)
now takes minutes to run the Makefile where before the make time was
negligible. One of our other builds went from ~1hr total to ~2.5hrs.

Maybe instead of exporting all the variables to `$(shell)` by default, we
could make it an opt-in feature? e.g., if `.SHELL_EXPORT := 1` is defined,
then the variables are all exported to the $(shell) invocation. This way
Makefiles that want to use this feature can declare that they need it in their
own Makefiles, while not impacting the performance of any older Makefiles.

Thanks,
Raul


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[anonymous]

Follow-up Comment #7, bug #63650 (project make):

I don't think it is reasonable to cache environment. The number of operations
needed for a single expansion is a very fast growing sequence.
So if it will help with e.g. n=5, n=6 will still take a long time to compute.

Maybe just show a warning when recursion is detected?

I'll try to think what can be done with algorithm.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Paul D. Smith]

Follow-up Comment #6, bug #63650 (project make):

I guess we might be saved by a weird behavior of GNU Make that I've long
considered changing because it confuses people: when we are about to expand a
recipe we expand all lines in the recipe at once, before we invoke the first
line.  This means that subsequent recipe lines can't observe changes to the
environment based on changes in the system that were created by prior recipe
lines, anyway.

In order for the behavior to be observed it would have to use *$(file ...)* in
one expansion followed by a *$(wildcard ...)*.  We could certainly make
expansion of *$(file ...)* be another operation that would invalidate the
environment.

I kind of hate to add anything that increases reliance on this unpleasant
(IMO) behavior of how recipes are expanded, however.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Paul D. Smith]

Update of bug #63650 (project make):

  Item Group: Bug => Enhancement

___

Follow-up Comment #5:

What about this idea: we would create a new environment when needed with all
variables expanded, as in current GNU Make 4.4.  Then instead of re-computing
it every time we want to run another command, we would only recompute it if
some make operation happened that might change it.  For example if we entered
a new rule (due to target-specific variables) or we modified a make variable
value, that would invalidate the environment.  If not, we could just re-use
the existing environment.

Running a shell command in itself, without setting a make variable, cannot
change the environment of a subsequent shell command.  At least, I can't think
of a way it could happen.

In that case, the example makefile below with the rule:

slow:
echo ${AAA}
echo ${AAB}
echo ${AAC}
echo ${AAD}
echo ${AAE}
echo ${AAF}

would still invoke the shell more often in order to compute the environment
for the first *echo* line above, but then, since none of these lines alter any
state in make itself, we could re-use that same environment.

But, actually maybe that's not good enough.  The value of the variable could
refer to functions like *$(wildcard ...)*, which would depend on the state of
the filesystem and so expand differently in the second line of the recipe than
they did for the first line of the recipe, if we didn't recompute the
environment.

Hm.  I'm going to convert this to an enhancement request.  I can't see a way
to fix it right now.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Paul D. Smith]

Follow-up Comment #4, bug #63650 (project make):

I don't think this is a bug.  Or, at least I don't think we can fix it.

In previous versions of GNU Make we did not add make variables to the
environment of the shell function regardless of their export status.  So for
example this makefile:

export FOO = bar
BAR = $(shell echo $$FOO)
all: ; @echo BAR=$(BAR)

would print *BAR=* in older versions of GNU Make.  In GNU Make 4.4 it will
print *BAR=bar*.


This was a problem for many reasons and was unexpected.  In GNU Make 4.4 we
changed this; from the NEWS file:

* WARNING: Backward-incompatibility!
  Previously makefile variables marked as export were not exported to
commands
  started by the $(shell ...) function.  Now, all exported variables are
  exported to $(shell ...).  If this leads to recursion during expansion,
then
  for backward-compatibility the value from the original environment is used.
  To detect this change search for 'shell-export' in the .FEATURES variable.


In your situation this means that every time make wants to invoke a shell
using the $(shell ...) function it must expand all the makefile variables (the
same way that it would when invoking a recipe).  If makefile variables take a
long time to expand (for example, they require invoking a lot of other shell
functions) you get this sort of pathological behavior.

I'm not sure I see any way to address this, other than writing more reasonable
makefiles.  For example if you use this instead:

AAA := $(shell echo 1)
AAB := $(shell echo 2)
AAC := $(shell echo 3)
AAD := $(shell echo 4)
AAE := $(shell echo 5)
AAF := $(shell echo 6)

then you won't see this problem because variables will only ever be expanded
one time.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Paul D. Smith]

Follow-up Comment #3, bug #63650 (project make):

I think they just mean they also tested Git HEAD (which is the commit
mentioned) and see the same behavior as with the GNU Make 4.4 release.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[Eli Zaretskii]

Follow-up Comment #2, bug #63650 (project make):

??? Do you mean to say that commit f51fc130 caused this regression?  If so, I
don't understand: that commit changed code that is only used on MS-Windows,
whereas the OP is on Arch Linux...



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[anonymous]

Follow-up Comment #1, bug #63650 (project make):

Update: reproduces when building from git. Commit sha f51fc130, message:
[SV 63638] Fix processing PATH on MS-Windows


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63650] Performance regression with EXPORT_ALL_VARIABLES enabled

[anonymous]

URL:
  

 Summary: Performance regression with EXPORT_ALL_VARIABLES
enabled 
 Project: make
   Submitter: None
   Submitted: Fri 13 Jan 2023 12:33:13 PM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: 4.4
Operating System: POSIX-Based
   Fixed Release: None
   Triage Status: None


___

Follow-up Comments:


---
Date: Fri 13 Jan 2023 12:33:13 PM UTC By: Anonymous
On fresh Arch Linux make takes 3.5s for the following makefile.


.EXPORT_ALL_VARIABLES:

AAA = $(shell echo 1)
AAB = $(shell echo 2)
AAC = $(shell echo 3)
AAD = $(shell echo 4)
AAE = $(shell echo 5)
AAF = $(shell echo 6)

slow:
echo ${AAA}
echo ${AAB}
echo ${AAC}
echo ${AAD}
echo ${AAE}
echo ${AAF}


Version: GNU Make 4.4
This reproduces with make compiled from sources from ftp.gnu.org.
The issue is not present in make 4.3.

make -d prints many lines like

Makefile:4: not recursively expanding AAB to export to shell function
Makefile:5: not recursively expanding AAC to export to shell function
Makefile:6: not recursively expanding AAD to export to shell function









___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/