bugmake - Bugs: bug #30328, Fast echo (with code proposal)

 
 

bug #30328: Fast echo (with code proposal)

Submitter:  None
Submitted:  Fri 02 Jul 2010 04:41:09 PM UTC
   
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  3.82 Operating System:  Any
Fixed Release:  4.0 Triage Status:  Medium Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 09 Oct 2013 05:18:33 AM UTC, comment #7: 

In the next release of GNU make we have a $(file ...) function.  There is also Guile integration as an extended scripting language, for more flexibility if necessary.

Paul D. Smith <psmith>
Group administrator
Mon 19 Jul 2010 05:37:41 PM UTC, comment #6: 

what about to have some function $(file file_name,open_type,string)

where open_type can be the same as for fopen



static void replace_all(char* inputStr,const char* pattern,const char* substr) {
   char * found = strstr(inputStr,pattern);
   int patternLen = strlen(pattern);
   int substrLen = strlen(substr);
   char * end = inputStr + strlen(inputStr);
   while (found != 0) {
      strncpy(found,substr,substrLen);
      memmove(found+substrLen,found+patternLen,end - (found + substrLen) + 1);
      found = strstr(found,pattern);
   }
}

/**
 * Output information to file.
 * argv - file name
 * argv + 1 - can be w,a, see fopen
 * argv + 2,... - message to output to file
*/
static char * func_file(char *o, char **argv, const char *funcname) {
   char **argvp;
   int len = 0;

   FILE * pFile = fopen(*argv,*(argv+1));
   if (pFile != 0) {
      for (argvp = argv+2; *argvp != 0; ++argvp) {
         char * param = *argvp;
         replace_all(param,"\\n","\n");
         replace_all(param,"\\t","\t");
         /* need to put more here */
         len += strlen(param) + 2;
         fputs(param,pFile);
      }
      fclose(pFile);
   } else {
      error(reading_file, _("Could not open file: `%s`"), *argv);
   }
   return o;
}


After that you can use to append


$(file file.out,a,some text with \n to append to file)


if you need to write to file


$(file file.out,w,some text with \n to create new file)


Olexiy Buyanskyy <olexiyb>
Mon 19 Jul 2010 01:58:49 PM UTC, comment #5: 

I'd like to add one point about this feature being extremely useful from a performance point of view.

I certainly have come across large makefiles where $(shell) for appending to a file is like crask - you desperately need to do it but it has a terrible effect on makefile parse time.

What I'd acutally like is a way to write functions as a loadable plugin so that we wouldn't have to ask for a GNU make feature every time we needed to do something special like this.

Timothy N Murphy <tnmurphy>
Mon 19 Jul 2010 01:51:42 PM UTC, comment #4: 

Make can handle simple quoting without falling back to the shell, including the difference between single- and double-quotes etc.

Paul D. Smith <psmith>
Group administrator
Mon 19 Jul 2010 11:01:18 AM UTC, comment #3: 

Hi Paul,

> Is it really true that this is an issue?


At least in our build there are "about to do this" and "having done that" log/trace echo commands in all recipes.

Using Cygwin with its high process creation overhead the proposed change gave a speedup in the high single-digit procent range.

> it doesn't deal with quotes


As far as I understand quotes make the command look something like 'sh.exe -c echo ...' while the proposed change checks for 'echo' (without the shell).

> it doesn't manage common options to echo (e.g., "-n")


That's true. But you stated yourself that quoting is the primary issue...

Anyway thanks for your consideration,
Oliver Schmidt - oliv.schmidt(at)sap.com

Anonymous
Sat 17 Jul 2010 03:45:13 PM UTC, comment #2: 

I am not sure if we really need echo, but it will be good to have command to output make variables to file.
Because of shell limitation that is very expensive with echo.

#performance is awful here!!!
all: $(thousands_of_target_files)
  $(foreach file,$^,echo $file >> list.all.targets)

I think that is very good to have additional parameter for existing $(info) function.

#This should work much faster
all: $(thousands_of_target_files)
  $(info $^,list.all.targets)

Olexiy Buyanskyy <olexiyb>
Fri 02 Jul 2010 06:20:41 PM UTC, comment #1: 

Is it really true that this is an issue?  The only "echo" commands that can be sped up are ones that appear on their own line in the recipe.  Any echo command that is on the same logical line as other commands would be passed to the same shell along with the other commands.

Also, the suggested code is not sufficient: it doesn't deal with quotes, and it doesn't manage common options to echo (e.g., "-n") or special characters (e.g., "\c").  Any built-in echo replacement must operate identically to the native echo--of course this is not really possible because echo is not standardized but at the least we should support quoting, which IS well-defined, plus any POSIX-required options.

Paul D. Smith <psmith>
Group administrator
Fri 02 Jul 2010 04:41:09 PM UTC, original submission:  

Hi,

Elaborated build systems based on GNU make tend to employ many 'echo' commands in their receipes for logging/tracing purposes.

These 'echo' commands (each and every creating a child process)introduce noticeable delay in those type of builds.

This can very easily be avoided by handling "those" 'echo' commands internally. Fortunately construct_command_argv() already differentiates between "those" simple logging/tracing 'echo' commands and 'echo' commands i.e. redirecting their output into files: If construct_command_argv() comes to the conclusion that 'echo' can be called directly without a shell then it is save to go one step further and avoid a child process altogether.

Please find attached the code to accomplish this. It is supposed to be inserted right after the (sort of similiar) optimization of empty commands in job.c.

Oliver Schmidt - oliv.schmidt(at)sap.com

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #20881:  echo.c added by None (439B - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by tnmurphy (Posted a comment)
  • -email is unavailable- added by olexiyb (Posted a comment)
  • -email is unavailable- added by psmith (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.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-10-09 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Component Version4.0 3.82
        Fixed ReleaseNone 4.0
    2010-07-02 psmith Triage StatusNone Medium Effort
    2010-07-02 None Attached File- Added echo.c, #20881

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code