[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] How to deal with LargeObjectException in RevWalk.parseCommit?

On Wed, Apr 4, 2012 at 14:57, Marc Strapetz <marc.strapetz@xxxxxxxxxxx> wrote:
> On 04.04.2012 20:31, Shawn Pearce wrote:
>> On Wed, Apr 4, 2012 at 11:24, Marc Strapetz <marc.strapetz@xxxxxxxxxxx> wrote:
>>>>
>>>> We could make this a repository level configuration setting, with the
>>>> default being the current 5 MiB. If users need it bigger, they could
>>>> set a larger limit in .git/config or ~/.gitconfig.
>>>
>>> OK. In addition I'd still add a setter to change the default limit, e.g.
>>> SmartGit usually runs with -Xmx256M, so I'd probably have 128M as
>>> default here. The few remaining users which use their pagefile.sys for
>>> the commit message will have to change their repository configuration.
>>> If there are no objections, I'll prepare a corresponding patch.
>>
>> It probably makes sense to connect this with the WindowCacheConfig. Or
>> maybe we just use core.streamFileThreshold here?
>>
>> Oh, now I remember. I wanted to make sure we could process most
>> commits, so I just hard coded this to 5 MiB. But we could do the
>> larger of 5 MiB or core.streamFileThreshold.
>>
>
> Having all these limits set to core.streamFileThreshold sounds
> reasonable. WindowCacheConfig is in a quite different package
> (storage.pack vs. revwalk), so I'd read that value directly from the
> Config.

This is a bit icky. You can't read it from the repository Config. The
WindowCacheConfig comes from a different configuration area than the
repository does. In EGit for example it comes from the workspace
settings as it applies to the entire JRE process, not an individual
repository that is being accessed. Because of this it doesn't make
sense to have a setter to set the default, your application would make
a new WindowCacheConfig, modify it as necessary, then apply it with
WindowCache.reconfigure(). You might do this immediately at
application startup before opening a repository for example.

Elsewhere we tend to get the limit from the ObjectReader. The
storage.file implementation of ObjectReader knows how to get the
streamFileThreshold from the WindowCache static members.

IIRC ObjectReader.getCachedBytes() the no argument form uses the
streamFileThreshold as the limit. Unfortunately we don't have a way to
say "the larger of 5M or the limit". Maybe we just use the no argument
form and tell anyone who sets their streamFileThreshold so small that
they can't load commits, tags or trees that they have to set it higher
to handle that repository?

So I am starting to think maybe the only JGit change here is to drop
that 5 * MiB argument in RevWalk, and your application just needs to
make sure you configure the WindowCache with what you determine are
reasonable settings.