Bug 353771

Comment 1 Shawn Pearce 2011-08-03 11:31:54 EDT

We should try to keep these optional for a while, so we can still support Java 5 and 6. Although 5 is starting to get pretty old. :-)

Comment 2 Robin Rosenberg 2013-01-13 18:34:18 EST

Experimental code at  https://git.eclipse.org/r/9642 .

With that Java7 status on my 70k file repo is ~20% faster than Java6.

Comment 3 Robin Rosenberg 2013-01-13 18:36:29 EST

.. and only 25% slower than native Git. On OS X by the way

Comment 4 Josef Stadelmann 2013-01-14 01:48:18 EST

yes but keep Java 6 for a while unless HP comes with a JAVA 7 for OpenVMS
OR
show how to endorse the required JAVA 7 jars for JGit to get what you want.

Comment 5 Robin Rosenberg 2013-01-14 04:14:12 EST

(In reply to comment #4)
> yes but keep Java 6 for a while unless HP comes with a JAVA 7 for OpenVMS
> OR
> show how to endorse the required JAVA 7 jars for JGit to get what you want.

If you are not using Java7, the Java7 feature will not be used. As simple as that.

-- robin

Comment 6 Josef Stadelmann 2013-01-14 04:27:49 EST

(In reply to comment #5)
> (In reply to comment #4)
> > yes but keep Java 6 for a while unless HP comes with a JAVA 7 for OpenVMS
> > OR
> > show how to endorse the required JAVA 7 jars for JGit to get what you want.
> 
> If you are not using Java7, the Java7 feature will not be used. As simple as
> that.
> 
> -- robin

Hopefully it works like that. But recently I was using JAXWS from Apache Axis2. AXIS2 supports JAXWS. And AXIS-2 as intended to run on JAVA 6. But one JAXWS annotation was calling via a AXIS2 side Interface a feature only implemented in JAVA 7 and so we got class not found errors. The cure was to endorse the right java 7 jar files implementig that feature. Not to say that this is a teddious task.

Josef

Comment 7 Robin Rosenberg 2013-01-14 04:56:47 EST

You'd better test that it works on openvms. I can't.

Comment 8 Alexander Riss 2013-01-28 05:54:39 EST

from my experience using the Files.exists and Files.getLastModifiedTime could also speed up all write operations to the index.

My profiler did show some heavy activity on File.exists and File.lastModified in ObjectDirectory / FileSnapshot.

These two calls were causing 90% of wait time on a DirCache.writeTree operation.

The NIO versions should be at lest twice as fast.

Comment 9 Robin Rosenberg 2013-01-28 06:21:46 EST

(In reply to comment #8)
> The NIO versions should be at lest twice as fast.

Did you try my patch?

Comment 10 Alexander Riss 2013-01-31 04:05:00 EST

i did not yet try out your patch since it does not affect the performance of a DirCache.writeTree call - as far as i could see from your changes.

I wrote a small patch for ObjectDirectory.hasObect2 and hasObject1 to use Files.exists and Files.getLastModifiedTime were possible. I saw a 100% performance bump for write operations (writing large DirCaches).

It seems there is a performance bug in hasObject1 which leads to rather poor performance when looking up objects from the index. My NIO version of hasObject2 now outperforms the index lookup of hasObject1....

Comment 11 Martin Oberhuber 2013-03-10 00:53:42 EST

Regarding fallback for not having Java 7 but still leveraging improved file system performance, see also my bug 354367 comment 11 .

Regarding the API additions in jgit FS, see also my comment on patch set 19
https://git.eclipse.org/r/#/c/9378/

Comment 12 Thomas Hallgren 2013-03-19 01:52:29 EDT

The patch moves JGit in a direction where it's no longer pure-java and have platform dependencies which in turn introduces increased build complexity etc. That's a bit dissapointing and IMO, it removes a very large part of the motivation behind JGit.

If it's a JNI solution you want, then why not simply provide a JNI layer on top of the native Git implementation instead? That way you'd get 95% less code to maintain.

Comment 13 Robin Rosenberg 2013-03-19 16:40:00 EDT

(In reply to comment #13)
> when running on Java 7 we could take advantage of new Java features to
> improve JGit/EGit:
> 
> - java.nio.file.attribute.BasicFileAttributeView could be used to improve
> performance of retrieving file attributes needed for git index

https://git.eclipse.org/r/#/c/9642/ does this.

Comment 14 Matthias Sohn 2014-02-11 19:39:56 EST

(In reply to Robin Rosenberg from comment #13)
> (In reply to comment #13)
> > when running on Java 7 we could take advantage of new Java features to
> > improve JGit/EGit:
> > 
> > - java.nio.file.attribute.BasicFileAttributeView could be used to improve
> > performance of retrieving file attributes needed for git index
> 
> https://git.eclipse.org/r/#/c/9642/ does this.

merged as 14cd43e6df8045c8f2879ddc8db0f59f45f2ce9c

Comment 15 David Ostrovsky 2014-02-12 09:36:36 EST

- implement AutoClosable interface for number of JGit classes, so that try-with-resources block can be used. So instead saying something like that:

 try {
      repo = gitManager.openRepository(change.getProject());
    } catch (RepositoryNotFoundException e) {
      throw new NoSuchChangeException(change.getId(), e);
    }
    try {
      RevWalk rw = new RevWalk(repo);
      RevCommit commit = edit.get(repo, rw);
      if (commit == null) {
        throw new NoSuchChangeException(change.getId());
      }
      BatchRefUpdate ru = repo.getRefDatabase().newBatchUpdate();
      ru.addCommand(new ReceiveCommand(commit,
          ObjectId.zeroId(), edit.toRefName()));
      try {
        ru.execute(rw, NullProgressMonitor.INSTANCE);
      } finally {
        rw.release();
      }
    } finally {
      repo.close();
   }

We can just say:

  try (Repository repo = gitManager.openRepository(change.getProject());
       RevWalk rw = new RevWalk(repo);
  ) {
     // use it
  }

Comment 16 Robin Rosenberg 2014-02-12 10:59:08 EST

(In reply to David Ostrovsky from comment #15)
> - implement AutoClosable interface for number of JGit classes, so that
> try-with-resources block can be used. So instead saying something like that:

That would tie us to Java 7. Today's solution uses Java 7 only if available.

Comment 17 Matthias Sohn 2014-02-12 12:28:55 EST

(In reply to Robin Rosenberg from comment #16)
> (In reply to David Ostrovsky from comment #15)
> > - implement AutoClosable interface for number of JGit classes, so that
> > try-with-resources block can be used. So instead saying something like that:
> 
> That would tie us to Java 7. Today's solution uses Java 7 only if available.

maybe we can provide some wrapper classes in the optional java 7 bundle ?

Comment 18 David Ostrovsky 2014-02-12 13:54:19 EST

(In reply to Robin Rosenberg from comment #16)
> (In reply to David Ostrovsky from comment #15)
> > - implement AutoClosable interface for number of JGit classes, so that
> > try-with-resources block can be used. So instead saying something like that:
> 
> That would tie us to Java 7. Today's solution uses Java 7 only if available.

Java 7 was released on July 28 2011.
Java 8 is expected next month, So i expect from the JGit library implementation of AutoClosable to make the code using JGit much more cleaner and stop copy/paste the whole boilerplates. Check Gerrit Code Review in general and this change in particular to see what i mean: [1].

If someone is going to use the next 10 years Java 5 and expect that the latest JGit library still supports it, well he has a problem then.  Today we have a problem: we are at Java 7 and ... don't have AutoClosable support in JGit library.

[1] https://gerrit-review.googlesource.com/#/c/52890/31/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionEditCommands.java

Comment 19 Robin Rosenberg 2014-02-12 14:13:24 EST

Please open a new issue. This issue is closed and fixed.

Comment 20 David Ostrovsky 2014-02-12 15:08:27 EST

(In reply to Robin Rosenberg from comment #19)
> Please open a new issue. This issue is closed and fixed.

Done in [1].

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=428039