Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-11 Thread Jakub Narebski
Derrick Stolee writes: > On 4/7/2018 2:40 PM, Jakub Narebski wrote: >> Derrick Stolee writes: >> >> [...] >>> On the Linux repository, performance tests were run for the following >>> command: >>> >>> git log --graph --oneline -1000 >>> >>>

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-09 Thread Stefan Beller
On Mon, Apr 9, 2018 at 6:15 AM, Derrick Stolee wrote: > I don't understand how folding the patches makes the correctness clearer, > since the rename (1/4) is checked by the compiler and the Coccinelle script > (3/4) only works after that rename is complete. > > The only thing I

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-09 Thread Derrick Stolee
On 4/8/2018 7:18 PM, Junio C Hamano wrote: Jeff King writes: If I were doing it myself, I probably would have folded patches 1 and 3 together. They are touching all the same spots, and it would be an error for any case converted in patch 1 to not get converted in patch 3. I'm

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-08 Thread Junio C Hamano
Jeff King writes: > If I were doing it myself, I probably would have folded patches 1 and 3 > together. They are touching all the same spots, and it would be an error > for any case converted in patch 1 to not get converted in patch 3. I'm > assuming you caught them all due to

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-07 Thread Derrick Stolee
On 4/7/2018 2:40 PM, Jakub Narebski wrote: Derrick Stolee writes: [...] On the Linux repository, performance tests were run for the following command: git log --graph --oneline -1000 Before: 0.92s After: 0.66s Rel %: -28.3% Adding '-- kernel/' to

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-07 Thread Jakub Narebski
Derrick Stolee writes: [...] > On the Linux repository, performance tests were run for the following > command: > > git log --graph --oneline -1000 > > Before: 0.92s > After: 0.66s > Rel %: -28.3% > > Adding '-- kernel/' to the command requires loading the

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-06 Thread Stefan Beller
On Fri, Apr 6, 2018 at 12:21 PM, Jeff King wrote: > On Fri, Apr 06, 2018 at 07:09:30PM +, Derrick Stolee wrote: > >> Derrick Stolee (4): >> treewide: rename tree to maybe_tree >> commit: create get_commit_tree() method >> treewide: replace maybe_tree with accessor methods

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-06 Thread Derrick Stolee
On 4/6/2018 3:21 PM, Jeff King wrote: On Fri, Apr 06, 2018 at 07:09:30PM +, Derrick Stolee wrote: Derrick Stolee (4): treewide: rename tree to maybe_tree commit: create get_commit_tree() method treewide: replace maybe_tree with accessor methods commit-graph: lazy-load trees for

Re: [PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-06 Thread Jeff King
On Fri, Apr 06, 2018 at 07:09:30PM +, Derrick Stolee wrote: > Derrick Stolee (4): > treewide: rename tree to maybe_tree > commit: create get_commit_tree() method > treewide: replace maybe_tree with accessor methods > commit-graph: lazy-load trees for commits I gave this only a

[PATCH v2 0/4] Lazy-load trees when reading commit-graph

2018-04-06 Thread Derrick Stolee
There are several commit-graph walks that require loading many commits but never walk the trees reachable from those commits. However, the current logic in parse_commit() requires the root tree to be loaded. This only uses lookup_tree(), but when reading commits from the commit- graph file, the