On Sat, May 12, 2018 at 04:13:59PM +0200, Duy Nguyen wrote: > > Should this one be global in the first place? Can we tie it to say > > "struct rev_info" or something? I'd somehow anticipate a far future > > where object flag bits used for traversal book-keeping would be moved > > out of the objects themselves and multiple simultanous traversal > > becomes reality. > > Yeah I thought about those 27 bits but discarded it because it was not > that much. But now that you brought up the maintainability aspect of > it, it might make sense to move those bits out (if we manage to make > commit-slab (or a specialized version of it) manage bitmaps instead of > primitive types, which is not impossible). > > I don't understand the tying to struct rev_info though. This is a > struct definition and cannot really be tied to another struct. The > pointer to struct source_slab _is_ tied to struct rev_info. Right. We have a global type name and global functions, because this is C. But the actual variable itself is inside struct rev_info (you used a pointer to a caller-allocated struct, but I think that's fine, too). > > Not limited to this particular one, but we'd need to think how the > > commit_slab mechanism should interact with the_repository idea > > Stefan has been having fun with. If the object pool is maintained > > per in-core repository object, presumably we'd have more than one > > in-core instances of the same commit object if we have more than one > > repository objects, and decorating one with a slab data may not want > > to decorate the other one at the same time. > > It should be ok. The slab is indexed by the "commit index" which is > already per parsed_object_pool. Commit-slab user has to be careful to > use the right slab with the right repo and free it at the right time, > but that I think is outside with struct repository. In theory somebody could misuse a "struct commit" from the wrong repository and get nonsense results. I'd like to think that would be pretty hard, but I guess it could be possible to make a mistake like that at the interface where we call into a submodule-related function. You could get around it by making the commit_index global across all pools, but I don't think that's a good idea. Since it's an array index, we want it to be compact and low. -Peff