+2005-04-12 Paul D. Smith <psmith@gnu.org>
+
+ The second expansion feature causes significant slowdown. Timing
+ a complex makefile (GCC 4.1) shows a slowdown from .25s to just
+ read the makefile before the feature, to 11+s to do the same
+ operations after the feature. Additionally, memory usage
+ increased drastically. To fix this I added some intelligence that
+ avoids the overhead of the second expansion unless it's required.
+
+ * dep.h: Add a new boolean field, need_2nd_expansion.
+
+ * read.c (eval): When creating the struct dep for the target,
+ check if the name contains a "$"; if so set need_2nd_expansion to 1.
+ (record_files): If there's a "%" in a static pattern rule, it gets
+ converted to "$*" so set need_2nd_expansion to 1.
+
+ * file.c (expand_deps): Rework to be more efficient. Only perform
+ initialize_file_variables(), set_file_variables(), and
+ variable_expand_for_file() if the need_2nd_expansion is set.
+
+ * implicit.c (pattern_search): Default need_2nd_expansion to 0.
+ (pattern_search): Ditto.
+ * main.c (handle_non_switch_argument): Ditto.
+ (main): Ditto.
+ * read.c (read_all_makefiles): Ditto.
+ (eval_makefile): Ditto.
+
2005-04-07 Paul D. Smith <psmith@gnu.org>
* main.c (main) [WINDOWS32]: Export PATH to sub-shells, not Path.
struct file *file;
unsigned int changed : 8;
unsigned int ignore_mtime : 1;
+ unsigned int need_2nd_expansion : 1;
};
static void
expand_deps (struct file *f)
{
- register struct dep *d, *d1;
+ struct dep *d, *d1;
struct dep *new = 0;
struct dep *old = f->deps;
unsigned int last_dep_has_cmds = f->updating;
+ int initialized = 0;
f->updating = 0;
f->deps = 0;
- /* We are going to do second expansion so initialize file
- variables for the file. */
- initialize_file_variables (f, 0);
-
for (d = old; d != 0; d = d->next)
{
if (d->name != 0)
{
char *p;
- struct dep **d_ptr;
- set_file_variables (f);
+ /* If we need a second expansion on these, set up the file
+ variables, etc. It takes a lot of extra memory and processing
+ to do this, so only do it if it's needed. */
+ if (! d->need_2nd_expansion)
+ p = d->name;
+ else
+ {
+ /* We are going to do second expansion so initialize file
+ variables for the file. */
+ if (!initialized)
+ {
+ initialize_file_variables (f, 0);
+ initialized = 1;
+ }
- p = variable_expand_for_file (d->name, f);
+ set_file_variables (f);
+
+ p = variable_expand_for_file (d->name, f);
+ }
/* Parse the dependencies. */
new = (struct dep *)
/* Files that follow '|' are special prerequisites that
need only exist in order to satisfy the dependency.
Their modification times are irrelevant. */
+ struct dep **d_ptr;
- struct dep *d;
for (d_ptr = &new; *d_ptr; d_ptr = &(*d_ptr)->next)
;
++p;
parse_file_seq (&p, '\0', sizeof (struct dep), 1),
sizeof (struct dep));
- for (d = *d_ptr; d != 0; d = d->next)
- d->ignore_mtime = 1;
+ for (d1 = *d_ptr; d1 != 0; d1 = d1->next)
+ d1->ignore_mtime = 1;
}
/* Enter them as files. */
else
free (d1->name);
d1->name = 0;
+ d1->need_2nd_expansion = 0;
}
/* Add newly parsed deps to f->deps. If this is the last
{
if (d->next == 0 && last_dep_has_cmds)
{
+ struct dep **d_ptr;
for (d_ptr = &new; *d_ptr; d_ptr = &(*d_ptr)->next)
- ;
+ ;
*d_ptr = f->deps;
f->deps = new;
}
else
{
- for (d_ptr = &(f->deps); *d_ptr; d_ptr = &(*d_ptr)->next)
+ struct dep **d_ptr;
+ for (d_ptr = &f->deps; *d_ptr; d_ptr = &(*d_ptr)->next)
;
*d_ptr = new;
}
}
- free_ns_chain ((struct nameseq*)old);
+ free_ns_chain ((struct nameseq *) old);
}
/* For each dependency of each file, make the `struct dep' point
void
snap_deps (void)
{
- register struct file *f;
- register struct file *f2;
- register struct dep *d;
- register struct file **file_slot_0;
- register struct file **file_slot;
- register struct file **file_end;
+ struct file *f;
+ struct file *f2;
+ struct dep *d;
+ struct file **file_slot_0;
+ struct file **file_slot;
+ struct file **file_end;
/* Perform second expansion and enter each dependency
name as a file. */
break; /* No more words */
/* If the dependency name has %, substitute the stem.
- Watch out, we are going to do something very smart
- here. If we just replace % with the stem value,
- later, when we do the second expansion, we will
- re-expand this stem value once again. This is not
- good especially if you have certain characters in
- your setm (like $).
+ Watch out, we are going to do something tricky here. If
+ we just replace % with the stem value, later, when we do
+ the second expansion, we will re-expand this stem value
+ once again. This is not good especially if you have
+ certain characters in your setm (like $).
- Instead, we will replace % with $* and allow the
- second expansion to take care of it for us. This
- way (since $* is a simple variable) there won't
- be additional re-expansion of the stem.*/
+ Instead, we will replace % with $* and allow the second
+ expansion to take care of it for us. This way (since $*
+ is a simple variable) there won't be additional
+ re-expansion of the stem. */
- for (p2 = p; p2 < p + len && *p2 != '%'; ++p2);
+ for (p2 = p; p2 < p + len && *p2 != '%'; ++p2)
+ ;
if (p2 < p + len)
{
dep = (struct dep *) xmalloc (sizeof (struct dep));
dep->ignore_mtime = d->ignore_mtime;
+ dep->need_2nd_expansion = 0;
s = d->name; /* Hijacking the name. */
d->name = 0;
if (recursions == 0)
struct dep *new = (struct dep *) xmalloc (sizeof (struct dep));
/* GKM FIMXE: handle '|' here too */
new->ignore_mtime = 0;
+ new->need_2nd_expansion = 0;
new->name = p = (char *) xmalloc (rule->lens[i] + fullstemlen + 1);
bcopy (rule->targets[i], p,
rule->suffixes[i] - rule->targets[i] - 1);
goals->next = 0;
goals->name = 0;
goals->ignore_mtime = 0;
+ goals->need_2nd_expansion = 0;
goals->file = default_goal_file;
}
}
lastgoal->name = 0;
lastgoal->file = f;
lastgoal->ignore_mtime = 0;
+ lastgoal->need_2nd_expansion = 0;
{
/* Add this target name to the MAKECMDGOALS variable. */
d->file = enter_file (*p);
d->file->dontcare = 1;
d->ignore_mtime = 0;
+ d->need_2nd_expansion = 0;
/* Tell update_goal_chain to bail out as soon as this file is
made, and main not to die if we can't make this file. */
d->changed = RM_DONTCARE;
filename = deps->file->name;
deps->changed = flags;
deps->ignore_mtime = 0;
+ deps->need_2nd_expansion = 0;
if (flags & RM_DONTCARE)
deps->file->dontcare = 1;
if (beg <= end && *beg != '\0')
{
+ char *top;
+ const char *fromp = beg;
+
+ /* Make a copy of the dependency string. Note if we find '$'. */
deps = (struct dep*) xmalloc (sizeof (struct dep));
deps->next = 0;
- deps->name = savestring (beg, end - beg + 1);
+ deps->name = top = (char *) xmalloc (end - beg + 2);
+ deps->need_2nd_expansion = 0;
+ while (fromp <= end)
+ {
+ if (*fromp == '$')
+ deps->need_2nd_expansion = 1;
+ *(top++) = *(fromp++);
+ }
+ *top = '\0';
deps->file = 0;
}
else
commands[commands_idx++] = '\n';
}
- /* Determine if this target should be made default. We used
- to do this in record_files() but because of the delayed
- target recording and because preprocessor directives are
- legal in target's commands it is too late. Consider this
- fragment for example:
+ /* Determine if this target should be made default. We used to do
+ this in record_files() but because of the delayed target recording
+ and because preprocessor directives are legal in target's commands
+ it is too late. Consider this fragment for example:
foo:
...
endif
- Because the target is not recorded until after ifeq
- directive is evaluated the .DEFAULT_TARGET does not
- contain foo yet as one would expect. Because of this
- we have to move some of the logic here. */
+ Because the target is not recorded until after ifeq directive is
+ evaluated the .DEFAULT_TARGET does not contain foo yet as one
+ would expect. Because of this we have to move some of the logic
+ here. */
if (**default_target_name == '\0' && set_default)
{
#ifdef HAVE_DOS_PATHS
&& strchr (name, '\\') == 0
#endif
- )
+ )
continue;
/* If there are multiple filenames, copy the chain DEPS
for all but the last one. It is not safe for the same deps
- to go in more than one place in the data base. */
+ to go in more than one place in the database. */
this = nextf != 0 ? copy_dep_chain (deps) : deps;
if (pattern != 0)
this = 0;
}
else
- {
- /* We use subst_expand to do the work of translating
- % to $* in the dependency line. */
+ /* We use subst_expand to do the work of translating % to $* in
+ the dependency line. */
- if (this != 0 && find_percent (this->name) != 0)
- {
- char *o;
- char *buffer = variable_expand ("");
+ if (this != 0 && find_percent (this->name) != 0)
+ {
+ char *o;
+ char *buffer = variable_expand ("");
- o = subst_expand (buffer, this->name, "%", "$*",
- 1, 2, 0);
+ o = subst_expand (buffer, this->name, "%", "$*", 1, 2, 0);
- free (this->name);
- this->name = savestring (buffer, o - buffer);
- }
- }
+ free (this->name);
+ this->name = savestring (buffer, o - buffer);
+ this->need_2nd_expansion = 1;
+ }
}
if (!two_colon)
/* This is the rule without commands. Put its
dependencies at the end but before dependencies
from the rule with commands (if any). This way
- everyhting appears in makefile order. */
+ everything appears in makefile order. */
if (f->cmds != 0)
{
/* This is a hack. I need a way to communicate to snap_deps()
that the last dependency line in this file came with commands
(so that logic in snap_deps() can put it in front and all
- this $< -logic works). I cannot's simply rely oon file->cmds
+ this $< -logic works). I cannot simply rely on file->cmds
being not 0 because of the cases like the following:
foo: bar
deps->next = 0;
deps->name = depname;
deps->ignore_mtime = 0;
+ deps->need_2nd_expansion = 0;
}
create_pattern_rule (names, percents, 0, deps, cmds, 0);