pack-objects: ignore ambiguous object warnings #67
Conversation
|
A few questions. |
|
@kewillford: We don't want to set this more generally, even for VFSforGit repos. This is an important warning for other scenarios. In this specific case, we know that |
|
We can't even assume |
|
Okay after taking a close look at all the code the method is taking the list of OIDs and some refs but uses the OIDs of those refs and this method is the only one that will use that pack-objects process. Thanks! |
kewillford
left a comment
kewillford
left a comment
There was a problem hiding this comment.
Are there any perf number you have for this?
|
We shouldn't set this repo wide as it would impact other commands the user directly initiated. I think the way you have it setup looks good. Perf numbers will help when you go to push this upstream. |
|
Here is the VFS for Git PR that incorporates this change as well as a |
|
Measuring the performance is quite noisy, especially with disk cache stuff making reruns much faster than first time. For my simple case of pushing an amended commit, I'm able to get ranges from 3.8s (warm cache, all changes) to 5.6s (cold cache, both changes), and 15s (cold cache, no changes) |
|
I submitted the patch upstream. Let's see what they think. |
derrickstolee
force-pushed
the
send-pack-config
branch
2 times, most recently
from
3cf6fa0 to
a14c312
Compare
derrickstolee
changed the title
derrickstolee
force-pushed
the
send-pack-config
branch
3 times, most recently
from
2e62055 to
002868e
Compare
A git push process runs several processes during its run, but one includes git send-pack which calls git pack-objects and passes the known have/wants into stdin using object ids. However, the default setting for core.warnAmbiguousRefs requires git pack-objects to check for ref names matching the ref_rev_parse_rules array in refs.c. This means that every object is triggering at least six "file exists?" queries. When there are a lot of refs, this can add up significantly! I observed a simple push spending three seconds checking these paths. The fix here is similar to 4c30d50 "rev-list: disable object/refname ambiguity check with --stdin". While the get_object_list() method reads the objects from stdin, turn warn_on_object_refname_ambiguity flag (which is usually true) to false. Just for code hygiene, save away the original at the beginning and restore it once we are done. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
derrickstolee
force-pushed
the
send-pack-config
branch
from
002868e to
a4544b3
Compare
|
Latest push is the commit that Junio queued. |
During a 'git push' command, we run 'git send-pack' inside of our transport helper. This creates a 'git pack-objects' process and passes a list of object ids. If this list is large, then the pack-objects process can spend a lot of time checking the possible refs these strings could represent. Remove this extra check by setting core.warnAmbiguousRefs to false as we call 'git pack-objects'. I was able to see the thousands of calls to different refs using a PerfView trace with "File I/O" selected. After this change, seconds of reading thousands of files was removed.
3 participants
During a 'git push' command, we run 'git send-pack' inside of our
transport helper. This creates a 'git pack-objects' process and
passes a list of object ids. If this list is large, then the
pack-objects process can spend a lot of time checking the possible
refs these strings could represent.
Remove this extra check by setting core.warnAmbiguousRefs to false
as we call 'git pack-objects'.
I was able to see the thousands of calls to different refs using a PerfView trace with "File I/O" selected. After this change, seconds of reading thousands of files was removed.