Shawn O. Pearce [Mon, 8 Aug 2011 22:11:54 +0000 (15:11 -0700)]
Speed up ObjectWalk by 6235 objects/sec
The "Counting objects" phase of packing is the most time consuming
part for any server providing access to Git repositories. Scanning
through the entire project history, including every revision of
every tree that has ever existed is expensive and takes an incredible
amount of CPU time.
Inline the tree parsing logic, unroll a number of loops, and setup
to better handle the common case of seeing another occurrence of
an object that was already marked SEEN.
This change boosts the "Counting objects" phase when JGit is acting
as a server and is packing the linux-2.6 repository for its client.
Compared to CGit on the same hardware, a JGit daemon server is now
21883 objects/sec faster:
CGit:
Counted 2058062 objects in 38981 ms at 52796.54 objects/sec
Counted 2058062 objects in 38920 ms at 52879.29 objects/sec
Counted 2058062 objects in 39059 ms at 52691.11 objects/sec
JGit (before):
Counted 2058062 objects in 31529 ms at 65275.21 objects/sec
Counted 2058062 objects in 30359 ms at 67790.84 objects/sec
Counted 2058062 objects in 30033 ms at 68526.69 objects/sec
JGit (this commit):
Counted 2058062 objects in 28726 ms at 71644.57 objects/sec
Counted 2058062 objects in 27652 ms at 74427.24 objects/sec
Counted 2058062 objects in 27528 ms at 74762.50 objects/sec
Above the first run was a "cold server". For JGit the JVM had just
started up with `jgit daemon`, and for CGit we hadn't touched the
repository "recently" (but it was certainly in kernel buffer cache).
The second and third runs were against the running JGit JVM, allowing
timing tests to better reflect the benefits of JGit's pack and index
caching, as well as any optimizations the JIT may have performed.
The timings are fair. CGit is opening, checking and mmap'ing both
the pack and index during the timer. JGit is opening, checking
and malloc+read'ing the pack and index data into its Java heap
during the timer. Both processes are walking the same graph space,
and are computing the "path hash" necessary to sort objects in the
object table for delta compression. Since this commit only impacts
the "Counting objects" phase, delta compression was obviously not
included in the timings and JGit may still be performing delta
compression slower than CGit, resulting in an overall slower server
experience for clients.
Change-Id: Ieb184bfaed8475d6960a494b1f3c870e0382164a Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Robin Stocker [Tue, 9 Aug 2011 21:31:50 +0000 (23:31 +0200)]
Add isSuccessful to MergeStatus, RebaseResult.Status and PullResult
This is useful when the result needs to be displayed and it's only of
interest if the operation was successful or not (in egit, it could be
used in MultiPullResultDialog).
Change-Id: Icfc9a9c76763f8a777087a1262c8d6ad251a9068 Signed-off-by: Robin Stocker <robin@nibor.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Shawn O. Pearce [Sat, 6 Aug 2011 01:18:16 +0000 (18:18 -0700)]
Fix offset64 creation for objects at 2 GiB
The offset32 format is used for objects <= 2^31-1, while the offset64
format is used for all other objects. This condition was missing
the = needed to ensure an object placed exactly at 2^31 would have
its 64 bit offset in the index.
Change-Id: I293fac0e829c9baa12cb59411dffde666051d6c5 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Mon, 8 Aug 2011 15:50:13 +0000 (08:50 -0700)]
PackWriter: Only search for base objects on thin packs
A non-thin pack does not need to worry about preferred bases, the pack
will be self-contained and all required delta base objects will appear
within the pack itself. Obtaining the path buffer and length from the
ObjectWalk to build the preferred base table is "expensive", so avoid
the cost unless a thin pack is being constructed.
Change-Id: I16e30cd864f4189d4304e7957a7cd5bdb9e84528 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Mon, 8 Aug 2011 15:31:30 +0000 (08:31 -0700)]
Fix jgit rev-list --objects master
This flag was not being honored due to a bug in createWalk().
argWalk is always non-null when there are commits passed in
on the command line. If --objects was specified, always make
a new ObjectWalk for the actual execution.
Change-Id: I6e1a1636f2634605d86671a83766cc1c42939821 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Sat, 6 Aug 2011 01:21:10 +0000 (21:21 -0400)]
Merge changes I58110f17,I440baa64,Ic77dcac5
* changes:
PackWriter: Skip progress messages on fast operations
IndexPack: Defer the "Resolving deltas" progress meter
IndexPack: Fix "Resolving deltas" progress meter
PackWriter: Skip progress messages on fast operations
If the "Finding sources" phase will complete in <1 second with no
delta compression enabled, don't bother showing the progress meter for
this phase. Small repositories on the local filesystem tend to rip
through this phase always subsecond and the ProgressMonitor display
can actually slow the operation down.
If delta compression is enabled, there are two phases that may run
very quickly. Set the timer to 500 milliseconds instead, reducing the
risk that the user has to wait longer than 1 second before any sort of
output from the packer occurs.
Change-Id: I58110f17e2a5ffa0134f9768b94804d16bbb8399 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
IndexPack: Defer the "Resolving deltas" progress meter
If delta resolution completes in < 1000 milliseconds, don't bother
showing the progress meter. This is actually very common for a Gerrit
Code Review server, where the client is probably sending 1 commit and
only a few trees/blobs modified... and the base objects are hot in the
process buffer cache.
The 1000 millisecond delay is just a guess at a reasonable time to wait.
Change-Id: I440baa64ab0dfa21be61deae8dcd3ca061bed8ce Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
This progress meter never reached 100% as it did not update while
resolving the external bases in thin packs.
Instead of updating in batches at the top level, update once per delta
that is resolved. The batching progress meter type should smooth out
the frequent updates to an update rate that is more reasonable to send
to the UI, while also ensuring a successful pack parse always reaches
100% deltas resolved.
Change-Id: Ic77dcac542cfa97213a6b0194708f9d3c256d223 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Wed, 29 Jun 2011 16:45:22 +0000 (09:45 -0700)]
Make PackReverseIndex a public structure
Repository inspection tools may find building a reverse index on a
pack useful, as they can then locate an object by offset. As both
C Git and JGit sometimes produce error messages with the offset
rather than the SHA-1, it may be useful to expose this type.
Some embeddings of UploadPack (e.g. Gerrit Code Review) set their own
PackConfig from a server-wide configuration, overriding any JGit
defaults or settings that may exist at the local repository level.
Make a copy constructor form of PackConfig so this server-wide
configuration object can be copied and then merged with repository
specific configuration data.
Change-Id: I4463c95aeaf7d6536c3ab132dec9c50ee528d9e0 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
The cached object databases should not require a close to release
their cached resources. Most object databases just return their
own reference for newCachedDatabase(), so a close() here kills
the real database's internal caches, and possibly underlying files,
resulting in poor performance for the callers of PackParser like
ReceivePack or FetchProcess trying to then go look up objects that
were just parsed, or that current references point to.
Change-Id: Ia4a239093866e5b9faf82744f729fb73f4373f1a Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Robin Rosenberg [Sun, 12 Jun 2011 22:20:51 +0000 (00:20 +0200)]
Fix reading of ref names containing characters that sort before /
A set of ref names like ('a/b' and 'a+b') would cause the RefDirectory
to think that the set of refs have changed because it traversed the
'a' directory in the subtree before looking at 'a+b', but it then
compared with the know refs which are sorted with 'a+b' first.
Fix this by traversing the refs tree in another order. Treat a directory
as if they ends with a '/' before deciding on the order to traverse
the refs tree.
Bug: 348834
Change-Id: I23377f8df00c7252bf27dbcfba5da193c5403917 Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Repository.writeMergeCommitMsg(null) no longer fails if the MERGE_MSG
file is missing. This was done to avoid CommitCommand to fail in case of
a missing MERGE_MSG file.
Sasa Zivkov [Tue, 5 Jul 2011 12:06:49 +0000 (14:06 +0200)]
Fixed creation of branch from a tag
Creation of a branch X from an annotated tag, as the starting point,
resulted into .git/refs/heads/X containing the ID of the annotated tag
instead of the ID of the tagged commit.
This fix peels the tag ref before using it as the starting point for
the newly created branch.
Bug: 340836
Change-Id: I01c7325770ecb37f5bf8ddb2a22f802466524f24 Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Adrian Goerler [Thu, 7 Jul 2011 21:16:40 +0000 (23:16 +0200)]
Cleanup directories leftover by test.
Use the temporary file management from superclass.
Change-Id: I3042951dc21860b4b85dd72a6bf41ee7cfe2aba4 Signed-off-by: Adrian Goerler <adrian.goerler@sap.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Adrian Goerler [Wed, 6 Jul 2011 21:10:44 +0000 (23:10 +0200)]
Cloning should fail when destination directory exists and is not empty
When trying to clone into a folder that already contains a cloned
repository native git will fail with a message "fatal: destination path
'folder' already exists and is not an empty directory.". Now JGit will
also fail in this situation throwing a JGitInternalException.
The test case was provided by Tomasz Zarna.
Bug: 347852
Change-Id: If9e9919a5f92d13cf038dc470c21ee5967322dac Also-by: Tomasz Zarna <Tomasz.Zarna@pl.ibm.com> Signed-off-by: Adrian Goerler <adrian.goerler@sap.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Shawn O. Pearce [Fri, 24 Jun 2011 19:55:19 +0000 (12:55 -0700)]
Improve performance when writing trees and small blobs
ObjectDirectoryInserter was always creating a temporary file,
writing the complete compressed contents of a tree, fsync()'ing
that to stable storage, and only then checking to see if there
was already an object with the same SHA-1 in the repository.
For commits this strategy makes some sense, the commit is very
unlikely to exist in the repository, as there are embedded times
and these change with each commit.
However for trees coming out of DirCache, it is more common for the
tree to already exist in the repository. Most subdirectories are
not modified in any given commit. Doing all of this local file IO
for things that already exist is very slow.
Try to detect cases where the object is "small enough" that it can
be processed entirely in memory, and avoid doing disk IO entirely
if the object already exists.
Also increase the size of the output buffer for the deflation.
This should boost the average write(2) syscall size from 512 bytes
to 8192 bytes, making streaming of large compressed contents to
disk slightly more efficient.
Change-Id: I1d40364e8725468522435814631916d73174c92b Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Fri, 27 May 2011 20:35:18 +0000 (13:35 -0700)]
Push errors back over sideband when possible
If an internal exception occurs while packing and the request
needs to abort, the HTTP response might already be committed due
to progress message having already been delivered to the client.
This prevents UploadPackServlet from resetting the response and
sending back an HTTP 500 response.
Try to catch all exceptions and report internal errors over the
sideband stream or as an ERR command during the initial ACK/NAK
negotiation phase. This allows JGit to transmit an error message
that the user will receive on their console without needing to
worry about resetting the (already gone) HTTP response.
Change-Id: Ie393fb8bb55d2b79ab1276adf71c781c1807f9fe Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Fri, 27 May 2011 17:46:02 +0000 (10:46 -0700)]
Report progress while updating references
If a fetch or push needs to apply more than a few references
to the local repository it may take more than 0.25 seconds to
process all of the updates. This is especially true in the DHT
storage system during an initial push of a project with many tags.
The backend database may need to use a transaction to ensure each
tag reference creation is unique, and there may be large delays
caused by these transactions.
Change-Id: Ib11a077adfbd525253e425d327f2e2c2380804c7 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Matthias Sohn [Thu, 9 Jun 2011 15:41:16 +0000 (17:41 +0200)]
Merge branch 'stable-1.0'
* stable-1.0:
Prepare post JGit v1.0.0.201106090707-r builds
JGit v1.0.0.201106090707-r
Include about.html files in maven build
Prepare post v1.0.0.201106081625-r builds
JGit v1.0.0.201106081625-r
Add missing about.html files to all shipped bundles
Prepare post v1.0.0.201106071701-r builds
JGit v1.0.0.201106071701-r
Matthias Sohn [Wed, 1 Jun 2011 23:45:50 +0000 (01:45 +0200)]
Merge branch 'stable-1.0'
* stable-1.0:
Prepare post v1.0.0.201106011211-rc3 builds
JGit v1.0.0.201106011211-rc3
Remove incubation marker
blame: Compute the origin of lines in a result file
Shawn O. Pearce [Fri, 27 May 2011 22:27:17 +0000 (15:27 -0700)]
blame: Compute the origin of lines in a result file
BlameGenerator digs through history and discovers the origin of each
line of some result file. BlameResult consumes the stream of regions
created by the generator and lays them out in a table for applications
to display alongside of source lines.
Applications may optionally push in the working tree copy of a file
using the push(String, byte[]) method, allowing the application to
receive accurate line annotations for the working tree version. Lines
that are uncommitted (difference between HEAD and working tree) will
show up with the description given by the application as the author,
or "Not Committed Yet" as a default string.
Applications may also run the BlameGenerator in reverse mode using the
reverse(AnyObjectId, AnyObjectId) method instead of push(). When
running in the reverse mode the generator annotates lines by the
commit they are removed in, rather than the commit they were added in.
This allows a user to discover where a line disappeared from when they
are looking at an older revision in the repository. For example:
blame --reverse 16e810b2..master -L 1080, org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
( 1080) } 2302a6d3 (Christian Halstrick 2011-05-20 11:18:20 +0200 1081) 2302a6d3 (Christian Halstrick 2011-05-20 11:18:20 +0200 1082) /** 2302a6d3 (Christian Halstrick 2011-05-20 11:18:20 +0200 1083) * Kick the timestamp of a local file.
Above we learn that line 1080 (a closing curly brace of the prior
method) still exists in branch master, but the Javadoc comment below
it has been removed by Christian Halstrick on May 20th as part of
commit 2302a6d3. This result differs considerably from that of C
Git's blame --reverse feature. JGit tells the reader which commit
performed the delete, while C Git tells the reader the last commit
that still contained the line, leaving it an exercise to the reader
to discover the descendant that performed the removal.
This is still only a basic implementation. Quite notably it is
missing support for the smart block copy/move detection that the C
implementation of `git blame` is well known for. Despite being
incremental, the BlameGenerator can only be run once. After the
generator runs it cannot be reused. A better implementation would
support applications browsing through history efficiently.
In regards to CQ 5110, only a little of the original code survives.
CQ: 5110
Bug: 306161
Change-Id: I84b8ea4838bb7d25f4fcdd540547884704661b8f Signed-off-by: Kevin Sawicki <kevin@github.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Shawn O. Pearce [Tue, 31 May 2011 16:14:50 +0000 (09:14 -0700)]
Merge branch 'stable-1.0'
* stable-1.0:
DHT: Support removing a repository name
DHT: Fix thread-safety issue in AbstractWriteBuffer
jgit.sh: Implement pager support
Change EditList to extend ArrayList
Ensure the HTTP request is fully consumed
Make sure test repositories are closed
Fix CloneCommand not to fetch into remote tracking branches when bare
Update Eclipse IP log for 1.0
Shawn O. Pearce [Fri, 27 May 2011 00:25:59 +0000 (17:25 -0700)]
DHT: Support removing a repository name
The first step to deleting a repository from the DHT storage is to
remove the name binding in the RepositoryIndexTable, making the
repository unavailable for lookup.
Change-Id: I469bf92f4bf2f555a15949569b21937c14cb142b Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Fri, 27 May 2011 00:19:30 +0000 (17:19 -0700)]
DHT: Fix thread-safety issue in AbstractWriteBuffer
There is a data corruption issue with the 'running' list if a
background thread schedules something onto the buffer while the
application thread is also using it.
Change-Id: I5ba78b98b6632965d677a9c8f209f0cf8320cc3d Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Sun, 29 May 2011 20:05:03 +0000 (13:05 -0700)]
jgit.sh: Implement pager support
If the command is either `diff` or `log`, there is often a lot of
lines of output. Run these commands through $GIT_PAGER, $PAGER, or
`less` in order to make it easier to browse the output on a terminal.
Change-Id: I18b87ea4acf404b94788f2ac2101812bd13e6a0f Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Sun, 29 May 2011 19:24:27 +0000 (12:24 -0700)]
Change EditList to extend ArrayList
There is no reason for this type to contain an ArrayList and try to
hide the implementation. It only slows down execution by adding an
extra layer of method dispatch to each invocation.
Instead subclass from ArrayList.
Change-Id: Ifbb9c7060c2fe3d5a7397c1aa85fbade14088637 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce [Fri, 27 May 2011 19:04:19 +0000 (12:04 -0700)]
Ensure the HTTP request is fully consumed
Some servlet containers require the servlet to read the EOF marker
from the input stream before a response can be output if the stream
is using "Transfer-Encoding: chunked"... which is typical for any
sort of large push to a repository over smart HTTP.
Ensure the EOF is always read by the PackParser when it is handling
the stream, and fail fast if there is more data present than expected
since this does indicate a protocol error.
Also ensure the EOF is read by UploadPack before it starts to output
a partial response using packing progress meters.
Change-Id: I131db9dea20b2324cb7c3272a814f21296bc64bd Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Some repositories created during tests are not added to the 'toClose'
list in LocalDiskRepositoryTestCase. Therefore when the tests end
we may have open FileHandles and on Windows this may cause the
tests to fail because we can't delete those files.
This is fixed by adding the possibility to explicitly add
repositories to the list of repos which are closed automatically.
Change-Id: I1261baeef4c7d9aaedd7c34b546393bfa005bbcc Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Fix CloneCommand not to fetch into remote tracking branches when bare
When cloning into a bare repository we should not create remote
tracking branches (e.g refs/remotes/origin/testX). Branches of the
remote repository should but fetched into into branches of the same
name (e.g refs/heads/testX). Also add the noCheckout option which
would prevent checkout after fetch.
Change-Id: I5d4cc0389f3f30c53aa0065f38119af2a1430909 Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
RefDirectory was not using FileSnapshot correctly in all places. This
is fixed with this commit. Additionally the constructors for the
different types of refs have been changed to take a FileSnapshot
instead of a modification time.
Change-Id: Ifb6a59e87e8b058a398c38cdfb9d648f0bad4bf8 Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Shawn O. Pearce [Fri, 13 May 2011 16:16:58 +0000 (09:16 -0700)]
DHT: Add sequence RefData
RefData now uses a sequence number as part of the field, ensuring
that updates always increase the sequence number by one whenever
a reference is modified.
Attaching a sequence number to RefData will help with storing
reference log entries during updates. As the sequence number should
be unique within the reference name space, log entries can be keyed
by the sequence number and remain unique. Making this work over
reference delete-create cycles will require an additional RefTable
API to return the oldest sequence number previously used in the
reference log to seed the recreated reference.
Change-Id: I11cfff2a96ef962e57f29925a3eef41bdbf9f9bb Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Shawn O. Pearce [Fri, 13 May 2011 14:44:42 +0000 (07:44 -0700)]
DHT: Replace TinyProtobuf with Google Protocol Buffers
The standard Google distribution of Protocol Buffers in Java is better
maintained than TinyProtobuf, and should be faster for most uses. It
does use slightly more memory due to many of our key types being
stored as strings in protobuf messages, but this is probably worth the
small hit to memory in exchange for better maintained code that is
easier to reuse in other applications.
Exposing all of our data members to the underlying implementation
makes it easier to develop reporting and data mining tools, or to
expand out a nested structure like RefData into a flat format in a SQL
database table.
Since the C++ `protoc` tool is necessary to convert the protobuf
script into Java code, the generated files are committed as part of
the source repository to make it easier for developers who do not have
this tool installed to still build the overall JGit package and make
use of it. Reviewers will need to be careful to ensure that any edits
made to a *.proto file come in a commit that also updates the
generated code to match.
CQ: 5135
Change-Id: I53e11e82c186b9cf0d7b368e0276519e6a0b2893 Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Shawn O. Pearce [Thu, 5 May 2011 18:18:54 +0000 (11:18 -0700)]
DHT: Remove per-process ChunkCache
Performance testing has indicated the per-process ChunkCache isn't
very effective for the DHT storage implementation. If a server is
using the DHT storage backend, it is most likely part of a larger
cluster where requests are distributed in a round-robin fashion
between the member servers.
In such a scenario there is insufficient data locality between
requests to get a good hit ratio on the per-process ChunkCache. A low
hit ratio means the cache is actually hurting performance by eating up
memory that could otherwise be used for transient request data, and
increasing pressure on the GC when it needs to find free space.
Remove all of the ChunkCache code. Installations that want to cache
(to reduce database usage) should wrap their Database with a
CacheDatabase and use a network based CacheServer.
I left the ChunkCache in the original DHT storage commit because I
wanted to document in the history of the project that its probably
worth *not* having, but leave open a door for someone to revert this
change if they find otherwise at a later date.
Change-Id: I364d0725c46c5a19f7443642a40c89ba4d3fdd29 Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Stefan Lay [Tue, 24 May 2011 08:38:59 +0000 (01:38 -0700)]
Add a DiffFormatter which calculates a patch-id
Adds a class which can be used to calculates a SHA1 of the diff
associated with a patch, similar to git patch-id.
In this version whitespace is not ignored.
Change-Id: I421d15ea905e23df543082786786841cbe3ef10d Signed-off-by: Stefan Lay <stefan.lay@sap.com> Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>