Gustaf Lundh [Fri, 1 Feb 2013 12:20:31 +0000 (13:20 +0100)]
Performance fixes in DateRevQueue
When a lot of commits are added to DateRevQueue, the
sort-on-insertion approach is very heavy on CPU cycles.
One approach to fix this was made by Dave Borowitz:
https://git.eclipse.org/r/#/c/5491/
But using Java's PriorityQueue seems to have brought some
extra overhead, and the desired performance could not be
reached.
This fix takes another approach to the insertion problem,
without changing the expected behaviour or bringing extra
memory overhead:
If we detect over 1000 commits in the DateRevQueue, a
"seek-index" is rebuilt every 1000th added commit.
The index keeps track of every 100th commit in the
DateRevQueue. During insertions, it will be used for a
preliminary scanning (binary search) of the queue, with
the intention of helping add() find a good starting point
to start walking from. After finding this starting point,
add() will step commit-by-commit until the correct
insertion place in the queue is found (today, the queue
is expected to be sorted at all times).
When applied to repositories with many refs, this approach
has proven to bring huge performance gains and scales quite
well.
For instance, in a repository with close to 80000 refs,
we could cut down the time a typical Gerrit replication
of 1 commit would take (just a push from JGit's point of
view) from 32sec down to 3.5sec.
Below you see some typical times to add a specific amount
of commits (with random commit times) to the DateRevQueue
and the difference the preliminary seek-index makes:
Commits | Index | No Index
1024 8ms 8ms
2048 13ms 9ms
4096 5ms 59ms
8192 11ms 595ms
16384 22ms 3058ms
32768 64ms 13811ms
65536 201ms 62677ms
131072 783ms 331585ms
Only one extra reference is needed for every 100 inserted
commits (and only when we see more than 1000 commits in
the queue), so the memory overhead should be negligible.
Various index-stepping values were tested, and 100 seemed to
scale very well and be effective from start.
In the future, it should probably be dynamic and based on
the number of refs in the queue, but this should serve well
as a starting point.
Note: While other fundamentally different data structures may
be more suitable, the DateRevQueue is extremely central to
many of the Git core operations. This approach was chosen,
since the effect of the patch is easy to predict in conjuction
with the current implementation. A totally new data structure
will make it harder to predict behaviour in many common and
uncommon cases (in terms of breaking ties, memory usage, cost
when using few elements, object creation/disposing overhead,
etc).
Robin Rosenberg [Thu, 24 Jan 2013 03:23:26 +0000 (04:23 +0100)]
Fix stash apply using merge logic
Instead of the complicated strange stuff, implement staah
apply as cherry-pick.
Provided there are no conflicts and it is requested that
the index should be applied, perform yet another cherry-pick,
but discard tha results thereof it that would result in conflicts.
Tobias Pfeifer [Thu, 17 Jan 2013 13:45:00 +0000 (14:45 +0100)]
Extract method to output the first header line of a git diff
In order to be able to determine the range of the first header line
(e.g. "diff --git a/file1 b/file2") in subclasses, the code that formats
the first header line is extracted.
Required by egit's change: Ia61398146c0336ab332234f24d341561292554db
Colby Ranger [Tue, 22 Jan 2013 22:54:05 +0000 (14:54 -0800)]
Reduce memory held and speed up DfsGarbageCollector.
getObjectList() returns a list of ObjectToPack. These can hold on to a
lot of memory. Furthermore, binary searching for objects in a sorted
array can be slow. Improve the speed and reduce the memory by creating a
copy of the ObjectId and inserting it into an ObjectIdOwnerMap.
Robin Stocker [Mon, 25 Jun 2012 22:55:39 +0000 (00:55 +0200)]
Enable marking entries using TreeFilters in DiffEntry
This adds a new optional TreeFilter[] argument to DiffEntry.scan. All
filters will be checked during the scan to determine if an entry should
be "marked" with regard to that filter.
After having called scan, the user can then call isMarked(int) on the
entries to find out whether they matched the TreeFilter with the passed
index.
An example use case for this is in the file diff viewer of EGit's
History view, where we'd like to highlight entries that are matching the
current filter.
See EGit change I03da4b38d1591495cb290909f0e4c6e52270e97f.
Bug: 393610
Change-Id: Icf911fe6fca131b2567514f54d66636a44561af1 Signed-off-by: Robin Stocker <robin@nibor.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Stefan Lay [Wed, 9 Jan 2013 13:02:51 +0000 (14:02 +0100)]
Add conflicts message before footer
In case of a conflict during cherry-pick or revert the commit message
was amended after the footer. This made the footer invalid. Many users
do not understand that they have to edit the commit message in order to
make it valid again.
Stefan Lay [Wed, 9 Jan 2013 13:02:13 +0000 (14:02 +0100)]
Only replace the ChangeId in the footer, not in the body
Additionally expose methods to find the first footer line and to find
the position of the ChangeId footer in the commit message in order to
enable reuse of these methods introduced for the fix.
Change-Id: I87ecca009ca3bff1ca0de3c436ebd95736bf5a10 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Markus Duft [Tue, 12 Jun 2012 05:02:58 +0000 (07:02 +0200)]
Fix patch application WRT windows line endings.
Previously the result of an application would have been \r\r\n in the
case of windows line endings, as RawText does not touch the \r, and
ApplyCommand adds "\r\n" if this is the ending of the first line in the
target file. Only always adding \n should be ok, since \r\n would be the
result if the file and the patch include windows line endings.
Colby Ranger [Sat, 19 Jan 2013 00:22:10 +0000 (16:22 -0800)]
Update DfsGarbageCollector to not read back a pack index.
Previously, the Dfs GC excluded objects from packs by passing a
previously written index to the PackWriter. Reading back a file on
Dfs is slow. Instead, allow the PackWriter to expose the objects
included in a pack and forward that to invocations of excludeObjects() .
Tomasz Zarna [Thu, 17 Jan 2013 20:38:43 +0000 (21:38 +0100)]
Add additional FastForwardMode enums for different config contexts
FastForwardMode is represented by different strings depending on context
it is set or get from. E.g. FastForwardMode.FF_ONLY for
branch.<name>.mergeoptions is "--ff-only" but for merge.ff it is "only".
Colby Ranger [Thu, 17 Jan 2013 23:00:38 +0000 (15:00 -0800)]
Rename PackConstants to PackExt, a typed pack file extension.
PackConstants previously contained string values for the pack and pack
index extension. Change PackConstant to be PackExt, a typed wrapper
around the string pack file extension.
Robin Rosenberg [Thu, 3 Jan 2013 23:24:48 +0000 (00:24 +0100)]
Define a tree filter for user-visible changes between two indexes
The primary purpose of the filter is to detect an index change that
could possibly lead to a change in what files are visible in the staging
view and decorations. Besides what TreeFilter.ANY_DIFF does for trees in
general, this filter also looks at the assume-valid (CE_VALID) flag to
see whether changes should be ignored or not.
Colby Ranger [Tue, 15 Jan 2013 22:56:48 +0000 (14:56 -0800)]
Remove getReverseIndexSize() from DfsPackDescription.
The method is used in only one location (DfsPackFile). Furthermore,
PackIndex already does an explicit computation of the size in
DfsPackFile. Simplify the DfsPackDescription by removing the method
and do the calculation similar to PackIndex.
Colby Ranger [Tue, 15 Jan 2013 22:24:30 +0000 (14:24 -0800)]
Use file extension with DfsPackDescription get/set file size.
Previously the size getters and setters had explicit methods for index
and pack. Update the api to be based on the file extension. This will
make it possible to support other extensions in the future, such as
the forthcoming bitmap extensions.
Roberto Tyley [Tue, 25 Dec 2012 09:10:51 +0000 (09:10 +0000)]
Fix concurrent creation of fan-out object directories
If multiple threads attempted to insert loose objects into the same new
fan-out directory, the creation of that directory was subject to a race
condition that could lead to an unnecessary IOException being thrown -
because an inserter could not 'create' a directory that had just been
generated by a different thread. All we require is that the directory
does indeed *exist*, so not being able to _create_ it is not actually a
fatal problem. Setting 'skipExisting' to 'true' on the call to mkdir()
fixes the issue.
I found this issue as a real world occurrence while working on The BFG
Repo Cleaner (https://github.com/rtyley/bfg-repo-cleaner), a tool which
concurrently performs a lot of object creation.
In order to demonstrate the problem here I've added a small test case
which reliably reproduces the issue on the few different hardware
systems I've tried. The error thrown when the race-condition arises is
this:
java.io.IOException: Creating directory /home/roberto/repo.git/objects/e6 failed
at org.eclipse.jgit.util.FileUtils.mkdir(FileUtils.java:182)
at org.eclipse.jgit.storage.file.ObjectDirectory.insertUnpackedObject(ObjectDirectory.java:590)
at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insertOneObject(ObjectDirectoryInserter.java:113)
at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(ObjectDirectoryInserter.java:91)
at org.eclipse.jgit.lib.ObjectInserter.insert(ObjectInserter.java:329)
Markus Duft [Fri, 13 Jul 2012 06:25:25 +0000 (08:25 +0200)]
Improve handling of checkout conflicts
This converts a checkout conflict exception into a RebaseResult /
MergeResult containing the conflicting paths, which enables EGit (or
others) to handle the situation in a user-friendly way
Shawn Pearce [Fri, 11 Jan 2013 22:42:35 +0000 (14:42 -0800)]
Accept '-' instead of space in enum config values
This is necessary because some versions of JGit containing
the flawed c98abc9c0586c73ef7df4172644b7dd21c979e9d were
used in the wild and wrote bad configuration files. We now
must accept this value in addition to the preferred case.
Colby Ranger [Thu, 10 Jan 2013 21:01:17 +0000 (13:01 -0800)]
Remove packIndex field from FileObjDatabase openPack method.
Previously, the FileObjDatabase required both the pack file path and
index file path to be passed to openPack(). A future change to add
a bitmap index will add a .bitmap file parallel to the pack file
(similar to the .idx file). Update the PackFile to support
automatically loading pack index extensions based on the pack file
path.
Colby Ranger [Thu, 10 Jan 2013 17:58:48 +0000 (09:58 -0800)]
Update DfsObjDatabase API to open/write by pack extension.
Previously, the DfsObjDatabase had a hardcoded getPackFile() and
getPackIndex() methods which opens a .pack and .idx file, respectively.
A future change to add a bitmap index will need to be stored in a
parallel .bitmap file. Update the DfsObjDatabase to support opening and
writing of files for any pack extension.
Dave Borowitz [Thu, 10 Jan 2013 18:53:54 +0000 (10:53 -0800)]
Peel tags during resolve of foo^
Once we start talking about parents of tags, we are in the commit
graph, so treat all objects from this point as commits. This fixes
spurious IncorrectObjectTypeExceptions on resolving expressions like
tag^^.
Matthias Sohn [Wed, 9 Jan 2013 12:56:32 +0000 (13:56 +0100)]
Delete the iplog generator
It stopped working when we moved to the Eclipse foundation's Gerrit
server since it doesn't use the Gerrit internal user store but LDAP.
Instead, since 2.0, we use the Eclipse foundation's automatic IP log
generator [1] to generate IP logs for releasing jgit and egit.
Tomasz Zarna [Fri, 7 Dec 2012 21:45:01 +0000 (22:45 +0100)]
Add additional FastForwardMode enums for different config contexts
FastForwardMode should be represented by different enums depending on
context it is set or get from. E.g. FastForwardMode.FF_ONLY for
branch.<name>.mergeoptions is "--ff-only" but for merge.ff it is "only".
Robin Rosenberg [Sun, 30 Dec 2012 19:57:27 +0000 (20:57 +0100)]
pgm: Attempt to detect a broken pipe and exit silently
When piping output to another program, the other pipe may exit
before we are done. An example is "jgit log|head". The result is
that errno get set to EPIPE. Unfortunately Java does not have
specific exception for this so we have to look at the exception
message and hope that the number of variants are small.
The detection here seem to work on Windows, Linux and OS X and it
seems the message is usually not localized.
Matthias Sohn [Fri, 4 Jan 2013 00:40:29 +0000 (01:40 +0100)]
Do not install unnecessary Orbit bundles
The org.eclipse.jgit.orbit.feature introduced recently in order to
install 3rd party Orbit dependencies is a too coarse grained approach
and has the effect to install some dependencies only needed for
org.eclipse.jgit.pgm when installing org.eclipse.jgit.feature.
Hence include the required 3rd party bundles into the respective jgit
features directly in order to avoid this problem.
Bug: 397356
Change-Id: I3a4a07de42808e787515a6865875d9513542d3f8 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Shawn Pearce [Tue, 1 Jan 2013 17:25:26 +0000 (12:25 -0500)]
Revert "Stop PathFilter after walking all matching paths"
This reverts commit 75eb6a147ffa8089e48d409f0215a16c755305e7.
Applications that want a PathFilter to abort the walk early should be
using PathFilterGroup. When a PathFilterGroup is created with exactly
one path its implementation is the same that 75eb6 tried to perform,
but has been long documented as having the behavior of breaking a
higher level OR filter graph node.
Robin Rosenberg [Tue, 23 Oct 2012 22:48:31 +0000 (00:48 +0200)]
Update the revert command and things relating to revert
Cherry-pick has been fixed, but even though revert does
basically the same thing, the fixes were not carried over here.
- Recognize the revert-states, analogous to the cherry picking states
- Make reset handle a revert-in-progress
- Update REVERT_HEAD and MERGE_MSG when revert fails due to conflicts
- Clear revert state on commit and reset
- Format the message similarily to how cherry-pick does. This is
not exactly how C Git does it.
The interface is still not the same as for cherry-picking.
Change-Id: I8ea956fcbc9526d62a2365360feea23a9280eba3 Signed-off-by: Chris Aniszczyk <zx@twitter.com>
Dave Borowitz [Thu, 27 Dec 2012 17:29:44 +0000 (09:29 -0800)]
Stop PathFilter after walking all matching paths
After the current path of the TreeWalk is no longer a prefix of the
PathFilter's path, there can be no more matching entries, but TreeWalk
will happily keep walking the rest of a (potentially very large and
recursive) tree unless StopWalkException is thrown. So, throw
StopWalkException from PathFilter.include() at the earliest
opportunity.
Colby Ranger [Thu, 27 Dec 2012 19:21:20 +0000 (11:21 -0800)]
Do not enforce DeltaWindow maxMemory when zero.
The maxMemory for a DeltaWindow can be optionally disabled when it is
less than or equal to zero. Respect this configuration when enforcing
the limits on object load.
Colby Ranger [Thu, 27 Dec 2012 16:48:39 +0000 (08:48 -0800)]
Enforce max memory for DeltaWindow.
Previously, memory limits were enforced at the start of each iteration
of the delta search, based on objects that were currently loaded in
memory. However, new objects added to the window may be expanded in a
future iteration of the search and thus were not accounted for correctly
at the start of the search. To fix this, memory limits are now enforced
before each object is loaded.
Robin Rosenberg [Sat, 15 Dec 2012 11:01:25 +0000 (12:01 +0100)]
Hide deprecation warnings in some test classes
These test classes heavily rely on Tree and associated classes. They
are convenient for building test cases and hence not yet replaced, but
there is a deprecation warning at about every line, which is not helpful.
Robin Rosenberg [Sun, 25 Nov 2012 16:17:20 +0000 (17:17 +0100)]
Mark non-externalizable strings as such
A few classes such as Constanrs are marked with @SuppressWarnings, as are
toString() methods with many liternal, but otherwise $NLS-n$ is used for
string containing text that should not be translated. A few literals may
fall into the gray zone, but mostly I've tried to only tag the obvious
ones.
Matthias Sohn [Sat, 29 Sep 2012 13:43:22 +0000 (15:43 +0200)]
Ensure that jgit p2 repository is self-contained
Add a new feature for the 3rd party dependencies jgit needs and include
this in org.eclipse.jgit.feature. This ensures that the jgit p2
repository is self-contained and downstream consumers don't need to find
the dependencies elsewhere. We don't add the jetty dependencies needed
to install the test feature org.eclipse.jgit.junit since this is only
used by egit tests.
Change-Id: I60425c746cd50f1875ef8597a8bbefb598b8c982 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Robin Rosenberg [Fri, 14 Dec 2012 23:30:42 +0000 (00:30 +0100)]
DirCacheIterator: Fix reset() and back()
reset() was broken and probably only worked when the position was
at the beginning. More serious was that back() sometimes descended
into the tree rather than skipping backward at the same level. Sometimes
this would result in false conflicts, but one could suspect silent
errors too. back() is called by the NamingConflictTreeWalk when looking
for directory/file conflicts.
Also added toString to DirCacheTree to simplify debugging.
Shawn Pearce [Fri, 7 Dec 2012 20:19:13 +0000 (12:19 -0800)]
Disable response compression on /git-receive-pack URLs
Compressing the response with gzip causes the stream to delay
flushing until gzip has seen the entire response message, or buffers
fill up and the compressed data has to be sent. This hides the
resolving progress monitor from the client, as well as any other
progress messages the server might be trying to send.
Disable compression in receive, matching what /git-upload-pack has.
projects / jgit.git / log