On Mon, Apr 28, 2014 at 03:35:02PM -0400, Jeff King wrote:

> Since such entries are in the minority, and because cache_entry is
> already a variable-length struct, I think you could get away with
> sticking it after the "name" field, and then comparing like:
>   const char *ce_normalized_name(struct cache_entry *ce, size_t *len)
>   {
>       const char *ret;
>       /* Normal, fast path */
>       if (!(ce->ce_flags & CE_NORMALIZED_NAME)) {
>               len = ce_namelen(ce);
>               return ce->name;
>       }
>       /* Slow path for normalized names */
>       ret = ce->name + ce->namelen + 1;
>       *len = strlen(name);
>       return ret;
>   }

That's the reading half. We would also need to create the normalized
names for each cache_entry. I took a look at that this afternoon. It
turns out we make cache_entry structs in quite a few places.  So I
thought I'd start with converting them all to a function like:

  struct cache_entry *cache_entry_alloc(const char *name, size_t len);

And then once converted, we could teach it to normalize the name as
appropriate. That interface does improve many of the callers, but there
are a few tricky ones.

For example, in checkout.c:update_some (and one or two other spots), we
actually have the path broken into two parts, and we combine them while
writing into the cache_entry. We could obviously combine them into a
single buffer beforehand, but that means extra copying in reasonably hot
code paths. It would be slightly ugly but perhaps reasonable to have:

  cache_entry_alloc_two(const char *one, size_t one_len,
                        const char *two, size_t two_len);

But then I got to unpack-trees. It has the whole path broken down across
a linked list.  I'm not sure what would be least terrible interface
here. Again, we could format to a buffer and copy, but I'm hesitant to
do it on this code path.

I'm not sure if I'm just being paranoid about the extra memcpys (it's
not _that_ tight a loop, since after all, we are generally zlib
inflating to get the tree data, and the filenames are not all _that_

I dunno. I just hate the idea of tradeoffs for this OS-X-only fix
permeating the rest of the code on other platforms. But maybe Knuth
should be hitting me with his premature optimization clue-stick.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to