Skip to content

Conversation

@ttaylorr
Copy link
Contributor

@ttaylorr ttaylorr commented Aug 7, 2017

This pull request teaches tq.(*TransferQueue).Add() to accept new items into the q.incoming queue of items while a batch is processing.

This is done to improve the performance of the 'delay' filter capability (see: #2466 for more). Prior to this pull request, a delayed checkout was performed like:

  1. Git and LFS handshake and negotiate supported capabilities.
  2. Git writes 100 items to LFS before LFS blocks while processing the items in the first batch.
  3. If there are more items in the checkout, see step 2.
  4. Checkout complete.

This means that we are doing nothing for however long it takes for Git to tell LFS about an entry in the checkout while a batch is processing.

Instead, accept new items while a batch is processing and save the time that it takes for Git to tell LFS about new items in the checkout. To do so, we keep a secondary batch called "pending", which is appended to from the <-q.incoming channel while a batch is processing. When a batch finishes processing, the union of "next" and "pending" are taken, with the first 100 elements becoming the next batch, and the rest are given back to "pending". This process repeats itself until there are no items remaining.

In #1758, I said that the benefit to having Add() be a blocking operation until an item was accepted into a new batch was:

Back-pressure. If the *TransferQueue can't accept more items, there's no sense in wasting a large amount of disk and CPU usage scanning the Git data. Previously, instances of git rev-list, git cat-file and git cat-file --batch would run ad nauseam even if the TransferQueue couldn't accept new items. Now, Add() will block after a maximum buffer depth has been reached. In other words, by default, Add() will accept 200 (twice the default batch size of 100) items before apply back-pressure up to the gitscanner, causing it to wait.

This statement is no longer true, since the amount of memory required per object is negligible as compared to the tq package as it was prior to #1758.


/cc @git-lfs/core
/cc #2466

@ttaylorr ttaylorr added the review label Aug 7, 2017
@ttaylorr ttaylorr added this to the v2.3.0 milestone Aug 7, 2017
@ttaylorr ttaylorr requested a review from rubyist August 7, 2017 20:38
@ttaylorr ttaylorr merged commit 8605c80 into master Aug 7, 2017
@ttaylorr ttaylorr deleted the tq-infinite-buffer branch August 7, 2017 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants