Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects
On Thu, Oct 31, 2013 at 3:07 AM, Jeff King wrote: > I think there are two cases that we need to consider: > > 1. We have a full repo and somebody requests a shallow clone for us. > We probably do not want to use bitmaps here. In the series we have > been testing, shallow clones turned off bitmaps because we do not > use the internal rev_list. But as of cdab485 (upload-pack: delegate > rev walking in shallow fetch to pack-objects), that distinction > doesn't hold. I think we can check the use of --shallow-file > instead of explicitly turning off bitmaps there. There's an (non-existing yet) case 1': somebody requests a clone and the source clone is already shallow. is_repository_shallow() could catch both cases. > 2. We have a shallow clone that wants to repack. We probably want to > turn off bitmap writing here. I don't think that grafts actually > matter here, because pack-objects should always be looking at the > true graph. It would mean that using "git rev-list > --use-bitmap-index" does not respect the grafts, and we should > probably disable it in that case (and ditto for replacements). Right. I forgot that the repo must be complete before it's grafted. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects
On Wed, Oct 30, 2013 at 05:28:08PM +0700, Nguyen Thai Ngoc Duy wrote: > > One other criterion I should have mentioned: we must be using the > > internal rev-list. That prevented us in v1.8.4.1 and earlier from using > > bitmaps for shallow fetches. But as of v1.8.4.2, we always use > > pack-objects' rev-walker. We may need to pass --no-use-bitmap-index for > > shallow fetches. > > I don't think a new option is needed. The code just needs to check if > there are any commit grafts. If there are, fall back to the old way. > That covers both shallow fetches and some rare grafted repos. I think > refs/replace/* does not impact rev walking, so we should be fine if > it's used. I think there are two cases that we need to consider: 1. We have a full repo and somebody requests a shallow clone for us. We probably do not want to use bitmaps here. In the series we have been testing, shallow clones turned off bitmaps because we do not use the internal rev_list. But as of cdab485 (upload-pack: delegate rev walking in shallow fetch to pack-objects), that distinction doesn't hold. I think we can check the use of --shallow-file instead of explicitly turning off bitmaps there. 2. We have a shallow clone that wants to repack. We probably want to turn off bitmap writing here. I don't think that grafts actually matter here, because pack-objects should always be looking at the true graph. It would mean that using "git rev-list --use-bitmap-index" does not respect the grafts, and we should probably disable it in that case (and ditto for replacements). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects
On Wed, Oct 30, 2013 at 2:36 PM, Jeff King wrote: > On Sat, Oct 26, 2013 at 05:25:14PM +0700, Nguyen Thai Ngoc Duy wrote: > >> > For bitmaps to be used, the following must be true: >> > >> > 1. We must be packing to stdout (as a normal `pack-objects` from >> > `upload-pack` would do). >> > >> > 2. There must be a .bitmap index containing at least one of the >> > "have" objects that the client is asking for. >> > >> > 3. Bitmaps must be enabled (they are enabled by default, but can be >> > disabled by setting `pack.usebitmaps` to false, or by using >> > `--no-use-bitmap-index` on the command-line). >> > >> > If any of these is not true, we fall back to doing a normal walk of the >> > object graph. >> >> I haven't read the bitmap creation code yet. But it probably does not >> matter. If the client requests a shallow fetch, you probably want to >> fall back to normal walk too. > > One other criterion I should have mentioned: we must be using the > internal rev-list. That prevented us in v1.8.4.1 and earlier from using > bitmaps for shallow fetches. But as of v1.8.4.2, we always use > pack-objects' rev-walker. We may need to pass --no-use-bitmap-index for > shallow fetches. I don't think a new option is needed. The code just needs to check if there are any commit grafts. If there are, fall back to the old way. That covers both shallow fetches and some rare grafted repos. I think refs/replace/* does not impact rev walking, so we should be fine if it's used. > As for repos that are themselves shallow, I do not know how doing a > "repack -b" would fare. Probably not well. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects
On Sat, Oct 26, 2013 at 05:25:14PM +0700, Nguyen Thai Ngoc Duy wrote: > > For bitmaps to be used, the following must be true: > > > > 1. We must be packing to stdout (as a normal `pack-objects` from > > `upload-pack` would do). > > > > 2. There must be a .bitmap index containing at least one of the > > "have" objects that the client is asking for. > > > > 3. Bitmaps must be enabled (they are enabled by default, but can be > > disabled by setting `pack.usebitmaps` to false, or by using > > `--no-use-bitmap-index` on the command-line). > > > > If any of these is not true, we fall back to doing a normal walk of the > > object graph. > > I haven't read the bitmap creation code yet. But it probably does not > matter. If the client requests a shallow fetch, you probably want to > fall back to normal walk too. One other criterion I should have mentioned: we must be using the internal rev-list. That prevented us in v1.8.4.1 and earlier from using bitmaps for shallow fetches. But as of v1.8.4.2, we always use pack-objects' rev-walker. We may need to pass --no-use-bitmap-index for shallow fetches. As for repos that are themselves shallow, I do not know how doing a "repack -b" would fare. Probably not well. > Bitmaps may be made work with shallow fetches too, I'm not sure. We > could substract the shallow'd commits out. The problem is if some > other commits share parts of the shallow'd commits, I'm not sure how > to detect that. Yeah, I do not think shallow follows the same have/want reachability rules that we rely on for taking the set difference via bitmaps. It may be made to work eventually, but I think the important thing at this point is making sure we properly fall back to the slow method when shallow. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects
On Fri, Oct 25, 2013 at 1:03 PM, Jeff King wrote: > From: Vicent Marti > > In this patch, we use the bitmap API to perform the `Counting Objects` > phase in pack-objects, rather than a traditional walk through the object > graph. For a reasonably-packed large repo, the time to fetch and clone > is often dominated by the full-object revision walk during the Counting > Objects phase. Using bitmaps can reduce the CPU time required on the > server (and therefore start sending the actual pack data with less > delay). > > For bitmaps to be used, the following must be true: > > 1. We must be packing to stdout (as a normal `pack-objects` from > `upload-pack` would do). > > 2. There must be a .bitmap index containing at least one of the > "have" objects that the client is asking for. > > 3. Bitmaps must be enabled (they are enabled by default, but can be > disabled by setting `pack.usebitmaps` to false, or by using > `--no-use-bitmap-index` on the command-line). > > If any of these is not true, we fall back to doing a normal walk of the > object graph. I haven't read the bitmap creation code yet. But it probably does not matter. If the client requests a shallow fetch, you probably want to fall back to normal walk too. Bitmaps may be made work with shallow fetches too, I'm not sure. We could substract the shallow'd commits out. The problem is if some other commits share parts of the shallow'd commits, I'm not sure how to detect that. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
