public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Cary Coutant <ccoutant@google.com>
To: Ian Lance Taylor <iant@google.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [gold patch] Incremental 15/18: Add --incremental-base option.
Date: Tue, 24 May 2011 22:20:00 -0000	[thread overview]
Message-ID: <BANLkTi=uPhqYhsbo0P6KHYwsaK90Qhcj=w@mail.gmail.com> (raw)
In-Reply-To: <mcr62p0jgbn.fsf@coign.corp.google.com>

>> @@ -861,6 +861,7 @@ Incremental_inputs::report_command_line(int argc, const char* const* argv)
>>         || strcmp(argv[i], "--incremental-changed") == 0
>>         || strcmp(argv[i], "--incremental-unchanged") == 0
>>         || strcmp(argv[i], "--incremental-unknown") == 0
>> +       || is_prefix_of("--incremental-base=", argv[i])
>>         || is_prefix_of("--debug=", argv[i]))
>>          continue;
>
> Will this work correctly if the user does
>    ld --incremental-base old-file
> ?  That is, if they pass the argument as a separate argv entry, rather
> than using =?  I guess the same question arises for --debug.

No, that's a known limitation. I plan to overhaul the command-line
comparison, and was intending to fix that issue then. I'd like to
store each argument as a separate string in the string table (where
many of the filename arguments probably already exist), and have an
array of pointers to the command-line options. That should reduce the
storage space required and eliminate the escaping of quotes.

Well, it's fairly trivial to go ahead and fix that now. How does the
following look, added immediately after the above if statement?

      if (strcmp(argv[i], "--incremental-base") == 0
	  || strcmp(argv[i], "--debug") == 0)
	{
	  // When these options are used without the '=', skip the
	  // following parameter as well.
	  ++i;
	  continue;
	}

>> +  // If the base file and the output file are different, open a
>> +  // new output file and copy the contents from the base file.
>> +  if (use_base_file)
>> +    {
>> +      unsigned char* base = this->base_;
>> +      this->open(s.st_size);
>> +      memcpy(this->base_, base, s.st_size);
>> +      ::munmap(base, s.st_size);
>> +      ::close(o);
>> +    }
>
> You're going to some mild contortions to map in the base file, and then
> you are throwing away the mapping.  If you are going to stick with this
> copying approach, then I think it would be simpler to just map the
> output file as for a non-incremental link, and then open() the base file
> and read() the contents into the mapping you just created.  That will be
> just as efficient--it's a nice sequential read--and will save you from
> running out of memory when you in effect map the output file twice.

Argh, good point. I'll rework this.

-cary

  reply	other threads:[~2011-05-24 22:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-04 23:30 Cary Coutant
2011-04-25 20:33 ` Cary Coutant
2011-05-24 14:59   ` Ian Lance Taylor
2011-05-24 22:20     ` Cary Coutant [this message]
2011-05-24 22:54       ` Cary Coutant
2011-05-24 23:08         ` Ian Lance Taylor
2011-05-24 23:33           ` Cary Coutant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='BANLkTi=uPhqYhsbo0P6KHYwsaK90Qhcj=w@mail.gmail.com' \
    --to=ccoutant@google.com \
    --cc=binutils@sourceware.org \
    --cc=iant@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).