On Tue, Feb 18, 2025 at 10:40:21AM -0800, Junio C Hamano wrote:
> > +static int get_commit_flags(struct commit *commit)
> > +{
> > + int *result = commit_flags_peek(&commit_flags, commit);
> > + return result ? *result : 0;
> > +}
> > +
> > +static void set_commit_flags(struct commit *commit, int flags)
> > +{
> > + *commit_flags_at(&commit_flags, commit) = flags;
> > +}
>
> ... it does not make much sense to use the slab mechanism if we are
> still limited to bitsizeof(type(flags)). If your "int" is 32 bit,
> and the command line fed 100 revs, we'd want a flags parameter that
> can house at least 100 bits passed into the "set" function, and
> yields the result that can hold at least 100 bits from the "get"
> function. I'd expect that the interface would be more like
>
> static int get_commit_flags(struct commit *commit, unsigned flags[]);
> static int set_commit_flags(struct commit *commit, unsigned flags[]);
>
> with a file-scope static variable signaling how many bits we are
> dealing with (allowing these functions to infer how many words
> flags[] array has), and the return values from these helpers to
> indicate success (with 0) and failure (with a negative value). On a
> 32-bit platform, you'd need at least 4 words in flags[] array to
> deal with 100 revs. On a 64-bit box, 2 words would be sufficient.
>
> I would imagine that we would need two more helpers
>
> static int set_flag_bit(unsigned flags[], unsigned n);
> static int get_flag_bit(unsigned flags[], unsigned n);
>
> to query the n-th bit in a set of bits stored in flags[] array.
Yeah. I did not see the word "stride" anywhere in your email, so I
wanted to provide a further hint: the commit-slab "_with_stride()"
variant is meant to handle this kind of arbitrary-sized data. This was
part of the original commit-slab implementation in a84b794ad0
(commit-slab: introduce a macro to define a slab for new type,
2013-04-13), but AFAIK we've never actually used it in practice. See
that commit for some examples.
-Peff
