Is packing thread-safe?

[danchr]

I'd like to add support for packing loose objects to hg-git. At the moment, all objects are written as loose objects when converting a Mercurial repository to Git, leading to excessively large and fragmented repositories.

Instead, I'd like to write the objects as packs so that they are compressed somewhat. The simplest solution is to write a single commit as a pack, but that has two downsides:

1. That's a lot of packs, and although it improves the situation somewhat, we can do better.
2. Packing is relatively slow, and thus the conversion becomes even slower.

Instead, I'd like to repack the repository while actually converting it. This appears to work well in practice, and the compression appears to allow use of multiple cores, but I wonder if its actually safe? Is there some reason that packing some objects while adding other objects to the repository wouldn't work?

In particular I intend to do something like this from multiple threads:

def _process_batch(object_store, shas):
    objects = {(object_store._get_loose_object(sha), None) for sha in shas}

    object_store.add_objects(list(objects))

    for obj, path in objects:
        object_store._remove_loose_object(obj.id)

Do you think that would be safe to invoke from multiple threads concurrently? Do you think it would conflict with the general conversion happening in another thread?

[jelmer]

This probably warrants a bit more thought, but here's an initial response:

So the good news is that if you go sufficiently lower down in the stack packing will be thread safe.

The code you provide is thread safe I believe, with two caveats:

Assuming that there is no overlap in objects that you want to pack (since otherwise one thread could call remove_loose_object for objects that another wants to call get_loose_object for). You can address this by calling object_store[sha] so that if the object has already been packed you retrieve the object from either the loose object or the pack.

Creating packs in parallel this way might also not be the most efficient way to do things, especially if you're creating loose objects that mirror objects already in packs.

[danchr]

Yeah, my initial version ran into issues trying to pack the same objects twice; that's why I essentially copied over pack_loose_objects(), and use a set to ensure I disregard any currently packing objects.

Nothing in hg-git — other than this code — really cares how the object_store manages objects and packs internally, so that should be fine.

Some numbers for the Dulwich repository:

• The current version creates a 150MB repository containing entirely loose objects in 25 seconds, using 93% CPU.
• Packing every one thousand commits creates a 68MB repository in 29 seconds, using 129% CPU.

I believe the benefits are greater for larger repositories, but it looks good. Writing and then reading should be relatively fast on an SSD, at least.

Thanks for the clarification 🙂

[jelmer]

We could potentially provide an API in Dulwich to allow packing with threading. Curious what a good signature for that function would be from your perspective though.

[danchr]

We could potentially provide an API in Dulwich to allow packing with threading. Curious what a good signature for that function would be from your perspective though.

You can see the basics of it in gc.py in hg-git: Essentially, for long runs there's a context manager within which you have the threads available. Then a final synchronous call occurs so that every export is converted into packs.

I plan on applying a bit further clean-ups, in particular so that logging and progress are fully abstracted so that it only happens on the main thread. And probably use dataclasses for the queues too.

I deliberately elected to use a regular interval of commits for packing, combined with a final “clean up”, as counting loose objects seems relatively slow in Dulwich. Ideally, it'd do some of the black magic in gc.c to make this faster, and I'd just do GC after every single commit. But that seems a tall order 🙂

EDIT: Issue #92 is relevant to this.