Community
Participate
Working Groups
This is an expensive operation in EGit, since EGit configures the WindowCache in its Activator, since it must set MMAP settings before any JGit operation trying to use them. Unfortunately, registering the MBean queries the user config to get the setting of jmx.StatsRecorder to figure out whether the MBean shall be registered at all. This adds significant time to Eclipse startup, in particular because it is typically the very first access to the user config, which will also check the system and jgit configs for being up to date, and if anything goes wrong, it prevents starting EGit. See https://www.eclipse.org/forums/index.php/t/1103963/ . The WindowCacheConfig should have a setting to prevent this automatic MBean registration, and WindowCache should have a public operation to allow registering that StatsRecorder later. Then EGit could set up this WindowCache in its Activator but activate this StatsRecorder and possibly have it registered as a JMX bean later, perhaps in a Job. Or perhaps not at all. This StatsRecorder seems to have been added in bug 553573 to diagnose problems in a Gerrit instance. While it might be interesting to monitor a running Eclipse instance via JMX, it's not really needed. There are other tools to diagnose memory problems in a local application like attaching a debugger or profiler.
New Gerrit change created: https://git.eclipse.org/r/163870
New Gerrit change created: https://git.eclipse.org/r/163873
Gerrit change https://git.eclipse.org/r/163870 was merged to [master]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=089eacb273e98b659d4f2c15721c1524e084ae07
Gerrit change https://git.eclipse.org/r/163873 was merged to [master]. Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=d5fbbe8ce118cc013c46772fff08a7ccdcb7b770
Unfortunately this isn't fixed yet. WindowCache contains a static block that forcibly configures it with default values, including exposeState = true: static { reconfigure(new WindowCacheConfig()); } So even if EGit configures it explicitly to not expose the statistics, the cache gets actually initialized twice: first with that default config with exposeStats=true; then again with the config provided by EGit. It's unclear to me how we could avoid this forced registration in a backwards-compatible way given this static initialization.
Created attachment 283788 [details] Patch
Same problem here. We are using a system property to skip the Monitoring (see patch in my previous comment).
Comment on attachment 283788 [details] Patch Same problem here. We are using a system property to skip the Monitoring (see attached patch).
Unfortunately a system property is not good for EGit. Perhaps some other hack though: set a static variable in some other class, before WindowCache.class is loaded, and make WindowCache check that variable. But that's sooo ugly. Other alternative: remove the static initialization and initialize on the first call to get(PackFile pack, long offset). But that introduces some synchronization there.
> Perhaps some other hack though: set a static variable in some other class, before WindowCache.class is loaded, and make WindowCache check that variable. But that's sooo ugly. I agree, that's quite ugly and it doesn't look robust. > Other alternative: remove the static initialization and initialize on the first call to get(PackFile pack, long offset). But that introduces some synchronization there. That sounds good to me. AtomicBoolean could be used instead of synchronization.
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/167263
(In reply to Eclipse Genie from comment #11) > New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/167263 Didn't get rid of the static init, but at least delays the potential MBean registration until first use of the WindowCache. This gives EGit and other clients a chance to re-configure not to do the registration before doing any JGit operations, while the behavior for other clients who relied on the default and did want the MBean registration is unchanged except that the bean may be registered a little later. Comes at an additional cost of an AtomicBoolean.getAndSet() on each call to WindowCache.get(PackFile, long), but I hope that's acceptable.
Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/167263 was merged to [master]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=71aeedb6ec79a91e41251a88c1ab3235c40a9b70
