Re: [PATCH] checkout: don't write merge results into the object database

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 15.06.2017 um 15:57 schrieb Jeff King:
> On Thu, Jun 15, 2017 at 01:33:42PM +0200, René Scharfe wrote:
> 
>> Merge results need to be written to the worktree, of course, but we
>> don't necessarily need object entries for them, especially if they
>> contain conflict markers.  Use pretend_sha1_file() to create fake
>> blobs to pass to make_cache_entry() and checkout_entry() instead.
> 
> Conceptually this makes sense, although I have a comment below.
> 
> Out of curiosity, did this come up when looking into the cherry-pick
> segfault/error from a few hours ago?

No, it came up in the discussion of Dscho's memory leak plug series [1].

>> @@ -225,8 +225,8 @@ static int checkout_merged(int pos, const struct checkout *state)
>>   	 * (it also writes the merge result to the object database even
>>   	 * when it may contain conflicts).
>>   	 */
>> -	if (write_sha1_file(result_buf.ptr, result_buf.size,
>> -			    blob_type, oid.hash))
>> +	if (pretend_sha1_file(result_buf.ptr, result_buf.size,
>> +			      OBJ_BLOB, oid.hash))
>>   		die(_("Unable to add merge result for '%s'"), path);
>>   	free(result_buf.ptr);
> 
> I wondered if pretend_sha1_file() makes a copy of the buffer, and indeed
> it does. So this is correct.
> 
> But that raises an interesting question: how big are these objects, and
> is it a good idea to store them in RAM? Obviously they're in RAM
> already, but I'm not sure if it's one-at-a-time. We could be bumping the
> peak RAM used if there's a large number of these entries. Touching the
> on-disk odb does feel hacky, but it also means they behave like other
> objects.
> 
> If it is a concern, I think it could be solved by "unpretending" after
> our call to checkout_entry completes. That would need a new call in
> sha1_file.c, but it should be easy to write.

Good point; we'd accumulate fake entries that we'll never need again.
The patch should clean them up.

Alternatively we could finally address the NEEDSWORK comment and
provide a way to checkout a file without faking an index entry..

René


[1] https://public-inbox.org/git/2704e145927c851c4163a68cfdfd5ada48fff21d.1493906085.git.johannes.schindelin@xxxxxx/



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]