From: Jeff King <peff@peff.net> To: Junio C Hamano <gitster@pobox.com> Cc: git@vger.kernel.org, "Jakub Narebski" <jnareb@gmail.com>, "Ted Ts\'o" <tytso@mit.edu>, "Jonathan Nieder" <jrnieder@gmail.com>, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, "Clemens Buchacher" <drizzd@aon.at>, "Shawn O. Pearce" <spearce@spearce.org> Subject: Re: [RFC/PATCHv2 2/6] add metadata-cache infrastructure Date: Wed, 13 Jul 2011 16:25:36 -0400 Message-ID: <20110713202536.GE31965@sigill.intra.peff.net> (raw) In-Reply-To: <7vipr66kmz.fsf@alter.siamese.dyndns.org> On Wed, Jul 13, 2011 at 12:33:24PM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > +`metadata_cache_lookup_uint32`:: > > +`metadata_cache_add_uint32`:: > > I think these are "uint31" functions, as you cannot signal missing entry > by returning a value with the MSB set if higher-end of uint32 range can be > a valid value. No, look at their definitions. I am careful not to assume we have a sentinel value for "not found", and you must use them like: uint32_t value; if (metadata_cache_lookup_uint32(c, obj, &value)) printf("value is %"PRIu32", value); else printf("value is not there"); > > + if (c->validity_fun) { > > + c->validity_fun(validity); > > + if (hashcmp(validity, p)) > > + return NULL; > > + } > > Two comments. > > - I would have expected that c->validity_check() would be a way for a > caller to implement a boolean function to check the validity of the > cache, with another hook c->validity_token() to generate/update the > token. I could then use the 20-byte space to store a timestamp and > check can say "It was still 3-days ago? fresh enough", or something > like that. But this is not a complaint--such a scheme I wrote in the > above four lines may be _too_ flexible to be useful. I started with exactly that interface, and came to the conclusion that it was unneeded flexibility. I can switch it back, but there's really not a need to at this point. The fact that there are 20 opaque bytes in the file is what will live with us from version to version. But if new code wants to be more flexible about how it checks the bytes, that is something that can be changed easily when the new code is added. > - I wonder if validity_fn() callback wants a callback parameter (the > pointer "c" itself, after adding an extra field to metadata_cache that > stores the callback parameter pointer of type "void *" and adding a > parameter to METADATA_CACHE_INIT() macro to initialize it). Neither this use nor my proposed patch-id cache would have a need for it, so I didn't bother. And like above, it can be easily changed later. > Other than that, this is looking fun ;-) Thanks. I'm pleased with the performance numbers I'm getting. I'm still a bit iffy on the alignment and aliasing issues in the first two patches. -Peff
next prev parent reply index Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-07-13 6:47 [RFC/PATCHv2 0/6] generation numbers for faster traversals Jeff King 2011-07-13 6:57 ` [RFC/PATCHv2 1/6] decorate: allow storing values instead of pointers Jeff King 2011-07-13 17:52 ` Jonathan Nieder 2011-07-13 20:08 ` Jeff King 2011-07-14 17:34 ` Jeff King 2011-07-14 17:51 ` [PATCH 1/3] implement generic key/value map Jeff King 2011-07-14 18:52 ` Bert Wesarg 2011-07-14 18:54 ` Bert Wesarg 2011-07-14 18:55 ` Jeff King 2011-07-14 19:07 ` Bert Wesarg 2011-07-14 19:14 ` Jeff King 2011-07-14 19:18 ` Bert Wesarg 2011-07-14 17:52 ` [PATCH 2/3] fast-export: use object to uint32 map instead of "decorate" Jeff King 2011-07-15 9:40 ` Sverre Rabbelier 2011-07-15 20:00 ` Jeff King 2011-07-14 17:53 ` [PATCH 3/3] decorate: use "map" for the underlying implementation Jeff King 2011-07-14 21:06 ` [RFC/PATCHv2 1/6] decorate: allow storing values instead of pointers Junio C Hamano 2011-08-04 22:43 ` [RFC/PATCH 0/5] macro-based key/value maps Jeff King 2011-08-04 22:45 ` [PATCH 1/5] implement generic key/value map Jeff King 2011-08-04 22:46 ` [PATCH 2/5] fast-export: use object to uint32 map instead of "decorate" Jeff King 2011-08-04 22:46 ` [PATCH 3/5] decorate: use "map" for the underlying implementation Jeff King 2011-08-04 22:46 ` [PATCH 4/5] map: implement persistent maps Jeff King 2011-08-04 22:46 ` [PATCH 5/5] implement metadata cache subsystem Jeff King 2011-08-05 11:03 ` [RFC/PATCH 0/5] macro-based key/value maps Jeff King 2011-08-05 15:31 ` René Scharfe 2011-08-06 6:30 ` Jeff King 2011-07-13 7:04 ` [RFC/PATCHv2 2/6] add metadata-cache infrastructure Jeff King 2011-07-13 8:18 ` Bert Wesarg 2011-07-13 8:31 ` Jeff King 2011-07-13 8:45 ` Bert Wesarg 2011-07-13 19:18 ` Jeff King 2011-07-13 19:40 ` Junio C Hamano 2011-07-13 19:33 ` Junio C Hamano 2011-07-13 20:25 ` Jeff King [this message] 2011-07-13 7:05 ` [RFC/PATCHv2 3/6] commit: add commit_generation function Jeff King 2011-07-13 14:26 ` Eric Sunshine 2011-07-13 7:05 ` [RFC/PATCHv2 4/6] pretty: support %G to show the generation number of a commit Jeff King 2011-07-13 7:06 ` [RFC/PATCHv2 5/6] check commit generation cache validity against grafts Jeff King 2011-07-13 14:26 ` Eric Sunshine 2011-07-13 19:35 ` Jeff King 2011-07-13 7:06 ` [RFC/PATCHv2 6/6] limit "contains" traversals based on commit generation Jeff King 2011-07-13 7:23 ` Jeff King 2011-07-13 20:33 ` Junio C Hamano 2011-07-13 20:58 ` Jeff King 2011-07-13 21:12 ` Junio C Hamano 2011-07-13 21:18 ` Jeff King 2011-07-15 18:22 ` Junio C Hamano 2011-07-15 20:40 ` Jeff King 2011-07-15 21:04 ` Junio C Hamano 2011-07-15 21:14 ` Jeff King 2011-07-15 21:01 ` Generation numbers and replacement objects Jakub Narebski 2011-07-15 21:10 ` Jeff King 2011-07-16 21:10 ` Jakub Narebski 2011-08-04 22:48 [RFC/PATCH 0/2] patch-id caching Jeff King 2011-08-04 22:49 ` [PATCH 1/2] cherry: read default config Jeff King 2011-08-04 22:49 ` [PATCH 2/2] cache patch ids on disk Jeff King 2011-08-04 22:52 ` Jeff King
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20110713202536.GE31965@sigill.intra.peff.net \ --to=peff@peff.net \ --cc=avarab@gmail.com \ --cc=drizzd@aon.at \ --cc=git@vger.kernel.org \ --cc=gitster@pobox.com \ --cc=jnareb@gmail.com \ --cc=jrnieder@gmail.com \ --cc=spearce@spearce.org \ --cc=tytso@mit.edu \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Mailing List Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/git/0 git/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 git git/ https://lore.kernel.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git