[m-rev.] for post-commit review: verbose_make_N_part_msg

[Zoltan Somogyi]

On 2023-07-11 10:45 +02:00 CEST, "Peter Wang" <novalazy at gmail.com> wrote:
>> ... the only computation in get_dependency_status of TargetFileName,
>> the call on what is now line 1389, was introduced almost a month earlier,
>> on June 1, in commit 083d376e65.
> 
> Yes, the regression was first introduced in commit 083d376e65.
> 
> But it is commit 9eb003458 that "forces" the call to
> get_make_target_file_name in get_dependency_status,
> by returning TargetFileName to the caller.

By "forcing", did you mean that

- without 9eb..., the compiler would, at the right optimization level,
  move the calls to get_make_target_file_name to just the branches
  of the if-then-elses and switches in get_dependency_state that
  need its output, which does NOT include the branch that is
  executed after the search in DepStatusMap0 succeeds, but

- with 9eb..., it can no longer do so?

If you did, you could have said so. I was looking at the code as it is
written, and arguing on that basis.

If you meant something else, then please tell me what you meant
in more detail.

>> Given this fact, and your pinpointing of this call as the cause of the slowdown,
>> may I presume that you have profiling data showing it as the bottleneck?
>> If yes, I would like to see it.
> 
> I timed the building of Prince with mmc --make:
>     - before and after commit 083d376e65
> or equivalently,
>     - commit 9eb003458
>     - commit 9eb003458 + the hack to return an invalid filename

Again, you could have said so, or wrote what you wrote in reply
to the email containing the diff for 083... Your email, by replying
to the email containing the diff for 9eb..., led me to believe
that the comparison was between compilers before and after
9eb...

And if commit 083... by itself led to the sixfold slowdown, then
why mention/blame 9eb...?

If those two times are all the profiling data you have, then
we *don't* know for sure that 9eb..'s change to get_dependency_status
was the only, or even the main, reason for the slowdown. Commit 083...
had other changes that could contribute some, or even most, of it.

Commit 083... was intended to reduce the maximum number of times
that we compute the file name for a make target from above one to just one.
Its aim was to avoid the possibility of inconsistency, not performance,
so I did not try to avoid cases where it could *raise* the number of times
that conversion was done from zero to one, which it seems you are saying
is the cause of the slowdown. I could go through 083... and look for places
where this could happen, and eliminate them. However, to avoid stepping
on each others' toes. I won't start on this while you are actively working on
the same code, so please tell me when you are done.

Zoltan.