bugmake - Bugs: bug #63650, Performance regression with...

 
 

bug #63650: Performance regression with EXPORT_ALL_VARIABLES enabled

Submitter:  None
Submitted:  Fri 13 Jan 2023 12:33:13 PM UTC
   
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Component Version:  4.4 Operating System:  POSIX-Based
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 14 Dec 2023 11:22:18 PM UTC, comment #9: 

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.)

Martin Dorey <mdorey>
Thu 14 Dec 2023 03:50:50 PM UTC, comment #8: 

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

Raul E Rangel <rrangel>
Thu 19 Jan 2023 09:06:35 AM UTC, comment #7: 

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.

Anonymous
Fri 13 Jan 2023 06:40:26 PM UTC, comment #6: 

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.

Paul D. Smith <psmith>
Group administrator
Fri 13 Jan 2023 06:17:18 PM UTC, 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.

Paul D. Smith <psmith>
Group administrator
Fri 13 Jan 2023 02:14:15 PM UTC, comment #4: 

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.

Paul D. Smith <psmith>
Group administrator
Fri 13 Jan 2023 01:49:01 PM UTC, comment #3: 

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.

Paul D. Smith <psmith>
Group administrator
Fri 13 Jan 2023 01:28:32 PM UTC, comment #2: 

??? 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...

Eli Zaretskii <eliz>
Group Member
Fri 13 Jan 2023 12:42:34 PM UTC, comment #1: 

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

Anonymous
Fri 13 Jan 2023 12:33:13 PM UTC, original submission:  

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


Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mdorey (Posted a comment)
  • -email is unavailable- added by rrangel (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by eliz (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-01-13 psmith Item GroupBug Enhancement

    Back to the top

    Powered by Savane 3.12.
    Corresponding source code