[PATCH 1 of 2 V2] pathencode.c: for long paths, strip first dir, whether or not it's "data/"
spectral at pewpew.net
Fri May 8 01:02:20 CDT 2015
On Thursday, May 7, 2015, Martin von Zweigbergk <martinvonz at google.com>
> On Thu, May 7, 2015 at 4:34 PM Martin von Zweigbergk <
> martinvonz at google.com
>> On Thu, May 7, 2015 at 4:27 PM Adrian Buehlmann <adrian at cadifra.com
>>> On 2015-05-08 00:56, Martin von Zweigbergk wrote:
>>> > Heh, Drew actually suggested "meta" just so we could avoid dealing with
>>> > this inconsistency between native and pure code.
>>> Yes, please.
>> I don't mind changing to "meta", but I'm still concerned about the
>> discrepancy between native and pure code. Do you have a test case that
>> shows that performance really is a problem?
> My crude timings suggest that the ~2k calls to hashencode() on the Firefox
> repo (~2k files out of ~200k have long names) take ~6ms, while the ~2k
> calls to strchr() take ~12us. So I'd say the performance argument does not
> exist and we just have to pick a name that we like.
> I don't particularly care what name we pick -- "metadata" and "meta" are
> both fine with me. Another obvious choice is "manifests", but I know Kyle
> (CC'd) prefers "metadata" in case we ever want to store something else per
> directory that's not a manifest. Even though we don't even have any plans
> for anything else, I see little argument for "manifests" over "meta(data)".
I'm fine with meta. As mentioned it has the benefit of being equal in
length, and so yeah you can copy the logic over to the pure code to just
strip 5. I agree that the discrepancy is the more annoying issue, so I'm
fine with whatever solution gets applied, just make them consistent.
> If you do, I could move the patch to the pure side (to strip exactly 5
>> characters). If not, I'd still prefer to have this patch applied, but it's
>> up to Matt in the end, of course.
> Matt, as explained above, I personally think this patch is still good.
>>> (I already like Drew)
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Mercurial-devel