Re: [PATCH v3 11/14] commit: integrate commit graph with commit parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/13/2018 7:12 PM, Jonathan Tan wrote:
On Thu,  8 Feb 2018 15:37:35 -0500
Derrick Stolee <stolee@xxxxxxxxx> wrote:

| Command                          | Before | After  | Rel % |
|----------------------------------|--------|--------|-------|
| log --oneline --topo-order -1000 |  5.9s  |  0.7s  | -88%  |
| branch -vv                       |  0.42s |  0.27s | -35%  |
| rev-list --all                   |  6.4s  |  1.0s  | -84%  |
| rev-list --all --objects         | 32.6s  | 27.6s  | -15%  |
Could we have a performance test (in t/perf) demonstrating this?

The rev-list perf tests are found in t/perf/p0001-rev-list.sh

The "log --oneline --topo-order -1000" test would be good to add to t/perf/p4211-line-log.sh

The "branch -vv" test is pretty uninteresting unless you set up your repo to have local branches significantly behind the remote branches. It depends a lot more on the data shape than the others which only need a large number of reachable objects.

One reason I did not use the builtin perf test scripts is that they seem to ignore all local config options, and hence do not inherit the core.commitGraph=true setting from the repos pointed at by GIT_PERF_REPO.


+static int check_commit_parents(struct commit *item, struct commit_graph *g,
+				uint32_t pos, const unsigned char *commit_data)
Document what this function does? Also, this function probably needs a
better name.

+/*
+ * Given a commit struct, try to fill the commit struct info, including:
+ *  1. tree object
+ *  2. date
+ *  3. parents.
+ *
+ * Returns 1 if and only if the commit was found in the commit graph.
+ *
+ * See parse_commit_buffer() for the fallback after this call.
+ */
+int parse_commit_in_graph(struct commit *item)
+{
The documentation above duplicates what's in the header file, so we can
probably omit it.

+extern struct object_id *get_nth_commit_oid(struct commit_graph *g,
+					    uint32_t n,
+					    struct object_id *oid);
This doesn't seem to be used elsewhere - do you plan for a future patch
to use it?




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux