From: Junio C Hamano <hidden> Date: 2016-06-15 22:57:08
Jeff King [off-list ref] writes:
On Sun, May 05, 2013 at 05:38:53PM -0500, Felipe Contreras wrote:
quoted
We don't care about blobs, or any object other than commits, but in
order to find the type of object, we are parsing the whole thing, which
is slow, specially in big repositories with lots of big files.
I did a double-take on reading this subject line and first paragraph,
thinking "surely fast-export needs to actually output blobs?".
Reading the patch, I see that this is only about not bothering to load
blob marks from --import-marks. It might be nice to mention that in the
commit message, which is otherwise quite confusing.
I had the same reaction first, but not writing the blob _objects_
out to the output stream would not make any sense, so it was fairly
easy to guess what the author wanted to say ;-).
I'm also not sure why your claim "we don't care about blobs" is true,
because naively we would want future runs of fast-export to avoid having
to write out the whole blob content when mentioning the blob again.
The existing documentation is fairly clear that marks for objects
other than commits are not exported, and the import-marks codepath
discards anything but commits, so there is no mechanism for the
existing fast-export users to leave blob marks in the marks file for
later runs of fast-export to take advantage of. The second
invocation cannot refer to such a blob in the first place.
The story is different on the fast-import side, where we do say we
dump the full table and a later run can depend on these marks.
By discarding marks on blobs, we may be robbing some optimization
possibilities, and by discarding marks on tags, we may be robbing
some features, from users of fast-export; we might want to add an
option "--use-object-marks={blob,commit,tag}" or something to both
fast-export and fast-import, so that the former can optionally write
marks for non-commits out, and the latter can omit non commit marks
if the user do not need them. But that is a separate issue.
From: Jeff King <hidden> Date: 2016-06-15 22:57:09
On Mon, May 06, 2013 at 08:08:45AM -0700, Junio C Hamano wrote:
quoted
I'm also not sure why your claim "we don't care about blobs" is true,
because naively we would want future runs of fast-export to avoid having
to write out the whole blob content when mentioning the blob again.
The existing documentation is fairly clear that marks for objects
other than commits are not exported, and the import-marks codepath
discards anything but commits, so there is no mechanism for the
existing fast-export users to leave blob marks in the marks file for
later runs of fast-export to take advantage of. The second
invocation cannot refer to such a blob in the first place.
OK. If the argument is "we do not write them, so do not bother reading
them back in", I think that is reasonable. It could hurt anybody trying
to run "fast-export" against a marks file created by somebody else, but
that is also the same case that is being helped here (since otherwise,
we would not be seeing blob entries at all).
I do not offhand know enough about the internals of import/export-style
remote-helpers to say whether the "hurt" case even exists, let alone how
common it is.
By discarding marks on blobs, we may be robbing some optimization
possibilities, and by discarding marks on tags, we may be robbing
some features, from users of fast-export; we might want to add an
option "--use-object-marks={blob,commit,tag}" or something to both
fast-export and fast-import, so that the former can optionally write
marks for non-commits out, and the latter can omit non commit marks
if the user do not need them. But that is a separate issue.
Yeah, that would allow the old behavior (and more) if anybody is hurt by
this. It is nice if the order of implementation is "more features, then
flip the default" because it provides an immediate escape hatch for
anybody who is hurt by the change in default. But again, I do not know
enough to say whether such hurt cases even exist.
-Peff
From: Felipe Contreras <hidden> Date: 2016-06-15 22:57:09
On Mon, May 6, 2013 at 10:08 AM, Junio C Hamano [off-list ref] wrote:
Jeff King [off-list ref] writes:
quoted
On Sun, May 05, 2013 at 05:38:53PM -0500, Felipe Contreras wrote:
quoted
We don't care about blobs, or any object other than commits, but in
order to find the type of object, we are parsing the whole thing, which
is slow, specially in big repositories with lots of big files.
I did a double-take on reading this subject line and first paragraph,
thinking "surely fast-export needs to actually output blobs?".
Reading the patch, I see that this is only about not bothering to load
blob marks from --import-marks. It might be nice to mention that in the
commit message, which is otherwise quite confusing.
I had the same reaction first, but not writing the blob _objects_
out to the output stream would not make any sense, so it was fairly
easy to guess what the author wanted to say ;-).
That's how fast-export has worked since --export-marks was introduced.
quoted
I'm also not sure why your claim "we don't care about blobs" is true,
because naively we would want future runs of fast-export to avoid having
to write out the whole blob content when mentioning the blob again.
The existing documentation is fairly clear that marks for objects
other than commits are not exported, and the import-marks codepath
discards anything but commits, so there is no mechanism for the
existing fast-export users to leave blob marks in the marks file for
later runs of fast-export to take advantage of. The second
invocation cannot refer to such a blob in the first place.
The story is different on the fast-import side, where we do say we
dump the full table and a later run can depend on these marks.
Yes, and gaining nothing but increased disk-space.
By discarding marks on blobs, we may be robbing some optimization
possibilities, and by discarding marks on tags, we may be robbing
some features, from users of fast-export; we might want to add an
option "--use-object-marks={blob,commit,tag}" or something to both
fast-export and fast-import, so that the former can optionally write
marks for non-commits out, and the latter can omit non commit marks
if the user do not need them. But that is a separate issue.
How? The only way we might rob optimizations is if there's an obscene
amount files, otherwise the number of blob marks that we are
*actually* going to use ever again is extremely tiny.
--
Felipe Contreras
From: Felipe Contreras <hidden> Date: 2016-06-15 22:57:09
On Mon, May 6, 2013 at 11:20 AM, Jeff King [off-list ref] wrote:
On Mon, May 06, 2013 at 08:08:45AM -0700, Junio C Hamano wrote:
quoted
quoted
I'm also not sure why your claim "we don't care about blobs" is true,
because naively we would want future runs of fast-export to avoid having
to write out the whole blob content when mentioning the blob again.
The existing documentation is fairly clear that marks for objects
other than commits are not exported, and the import-marks codepath
discards anything but commits, so there is no mechanism for the
existing fast-export users to leave blob marks in the marks file for
later runs of fast-export to take advantage of. The second
invocation cannot refer to such a blob in the first place.
OK. If the argument is "we do not write them, so do not bother reading
them back in", I think that is reasonable.
We already do that:
5d3698f fast-export: avoid importing blob marks
It could hurt anybody trying
to run "fast-export" against a marks file created by somebody else, but
that is also the same case that is being helped here (since otherwise,
we would not be seeing blob entries at all).
I do not offhand know enough about the internals of import/export-style
remote-helpers to say whether the "hurt" case even exists, let alone how
common it is.
quoted
By discarding marks on blobs, we may be robbing some optimization
possibilities, and by discarding marks on tags, we may be robbing
some features, from users of fast-export; we might want to add an
option "--use-object-marks={blob,commit,tag}" or something to both
fast-export and fast-import, so that the former can optionally write
marks for non-commits out, and the latter can omit non commit marks
if the user do not need them. But that is a separate issue.
Yeah, that would allow the old behavior (and more) if anybody is hurt by
this.
There is no behavior change in this patch. We do *exactly* the same as before.
--
Felipe Contreras