jonashaag / klaus

docker run klaus / pip install klaus — the first Git web viewer that Just Works™.
http://klausdemo.lophus.org
Other
687 stars 101 forks source link

very slow with 10k tags #248

Open mgaunard opened 4 years ago

mgaunard commented 4 years ago

Loading every single page takes 20-30 seconds. If I remove the get tags function from the context it only takes a couple of seconds.

Tags don't seem that important, how come they have such a terrible performance impact? Could some caching logic help?

jonashaag commented 4 years ago

I'd be interesting in speeding things up. Can you provide an example repo that I can use to reproduce the issue?

You can also simply disable ctags for the time being.

jelmer commented 4 years ago

Is this about git tags or ctags?

mgaunard commented 4 years ago

This is about git tags. Nothing to do with ctags.

jonashaag commented 4 years ago

I did some investigation today. We have multiple causes here.

The time spent compiling the list of tags (and branches) for display is spent 50% on listing refs (Dulwich’s refs.as_dict()), and 50% on looking up each ref’s time stamp for sorting.

Then, looking up refs and checking their time stamps is not cached at the moment.

We can easily cache the time stamp part; there’s already logic in the code base for cache invalidation based on changed list-of-refs. The ref lookup would still need to be performed on each page load, since it is used as cache validator, so we can only save 50% of page load time using this method.

@jelmer two Dulwich/Git questions here:

jonashaag commented 4 years ago

Btw, Mathias, if you have no more than 10k tags and page load takes up to 30 seconds, are you using a terribly slow file system or operating system or a very slow computer? My benchmarks were more in the range of 1-2 seconds for 10k tags on a moderately capable MacBook.

jelmer commented 4 years ago

On Tue, Mar 17, 2020 at 01:55:35PM -0700, Jonas Haag wrote:

I did some investigation today. We have multiple causes here.

The time spent compiling the list of tags (and branches) for display is spent 50% on listing refs (Dulwich’s refs.as_dict()), and 50% on looking up each ref’s time stamp for sorting.

Then, looking up refs and checking their time stamps is not cached at the moment.

We can easily cache the time stamp part; there’s already logic in the code base for cache invalidation based on changed list-of-refs. The ref lookup would still need to be performed on each page load, since it is used as cache validator, so we can only save 50% of page load time using this method.

@jelmer two Dulwich/Git questions here:

  • Is there a faster way to check for changed refs (tags and branches) than compiling the dict of refs? Probably we can use ref files’ modification time stamps as a cheap has-any-ref-been-modified pre-check, and only then resolve each ref to check for changes? Is this a reliable method?
  • Is there a faster way to look up a bunch of refs at the same time? Use case would be looking up all refs’ time stamps for sorting. Packing the refs into a packed-refs file would be a good alternative that is probably much faster to parse. That does however involve write access to the repository.

Another option may be to provide notifications for tag changes on top of inotify, either inside of dulwich or inside of klaus.

I'm a little wary of tracking file timestamps; the refs files themselves are tiny, and the overhead of tracking timestamps versus actually reading the files is also non-trivial (both in terms of performance and in terms of additional code).

Cheers,

Jelmer

-- Jelmer Vernooij jelmer@jelmer.uk PGP Key: https://www.jelmer.uk/D729A457.asc

jonashaag commented 4 years ago

Thanks!

Me: Is there a faster way to look up a bunch of refs at the same time? Use case would be looking up all refs’ time stamps for sorting.

Do you think we could gain some speedup by batching the ref lookups?

the overhead of tracking timestamps versus actually reading the files is also non-trivial (both in terms of performance and in terms of additional code).

Code-wise, isn't it only a matter of calling os.stat() on each ref file? Do you think that's slower than reading each file's contents?

jelmer commented 4 years ago

On Wed, Mar 18, 2020 at 05:52:17AM -0700, Jonas Haag wrote:

Me: Is there a faster way to look up a bunch of refs at the same time? Use case would be looking up all refs’ time stamps for sorting. Do you think we could gain some speedup by batching the ref lookups? Not by much - the performance of Refs.as_dict() can probably be improved somewhat, but not by much. packed-refs will make a big difference here, rather than using individual files.

the overhead of tracking timestamps versus actually reading the files is also non-trivial (both in terms of performance and in terms of additional code). Code-wise, isn't it only a matter of calling os.stat() on each ref file? Do you think that's slower than reading each file's contents? statting 10k files is not fast either; I'm not convinced it's going to be a significant win over just reading the file contents.

I think we can add a mechanism in Dulwich that allows you to listen to updates to refs - I'ved filed https://github.com/dulwich/dulwich/issues/751 about this.

Jelmer

-- Jelmer Vernooij jelmer@jelmer.uk PGP Key: https://www.jelmer.uk/D729A457.asc

jelmer commented 4 years ago

You can now use RefsContainer.watch() to wait for changes to refs:

r = Repo('.')
with r.refs.watch() as w:
    for (ref, sha) in w:
        print('Ref %s has been updated to %s' % (ref, sha))

Note that you'd probably have to run this from another thread, since it will block until one of the refs changes.

jonashaag commented 4 years ago

Great news!

Minor downer is that inotify is Linux-only, so no macOS or BSD support. But that’s probably easy to add, and maybe entirely unnecessary since 99% of users with huge number of repositories will run Linux anyways.

jelmer commented 4 years ago

The API on the Dulwich side should be generic enough that we could add a Windows-specific implementation (perhaps using watchdog?) to it, without any changes on the klaus side.