Re: [PATCH v2 1/5] histograms: Initial histograms interface

[Stevie Alvarez <stevie.6strings@gmail.com>]

On Fri, Aug 04, 2023 at 01:57:18PM -0400, Steven Rostedt wrote:
> On Fri, 4 Aug 2023 13:41:59 -0400
> Stevie Alvarez <stevie.6strings@gmail.com> wrote:
> 
> > > > +struct traceeval_type {
> > > > +	char				*name;
> > > > +	traceeval_dyn_release_fn	dyn_release;
> > > > +	traceeval_dyn_cmp_fn		dyn_cmp;
> > > > +	enum traceeval_data_type	type;
> > > > +	size_t				flags;
> > > > +	size_t				id;  
> > > 
> > > Let's reorder this a little. Normally function pointers come at the end of
> > > a structure. That's more of a guideline than a rule, but let's have it here.
> > > 
> > > struct traceeval_type {
> > > 	char				*name;
> > > 	enum traceeval_data_type	type;
> > > 	size_t				flags;
> > > 	size_t				id;
> > > 	traceeval_dyn_release_fn	dyn_release;
> > > 	traceeval_dyn_cmp_fn		dyn_cmp;
> > > };
> > > 
> > > Especially since dynamic types are going to be rare, we don't want it in
> > > the hot cache.  
> > 
> > Does the order of the fields in a struct definition not matter? I
> > thought word-boundaries applied to struct definitions? Or does the
> > compiler take care of this?
> 
> They do matter. Word bounders are important, but the compiler will just
> make "holes" if needed. For example, let's say on 64 bit, everything above
> is 64 bits but the type. I would have created a "hole". But because the
> type is more important than the id, I kept it at the top.
> 
> struct traceeval_type {
> 	char				*name;			// offset 0
>  	enum traceeval_data_type	type;			// offset 8
> 
> [ compiler adds 4 byte "hole" or "padding" ]
> 
>  	size_t				flags;			// offset 16
>  	size_t				id;			// offset 24
>  	traceeval_dyn_release_fn	dyn_release;		// offset 32
>  	traceeval_dyn_cmp_fn		dyn_cmp;		// offset 40
> };
> 
> If a cache line is 32 bytes (most is usually 128, but let's say on an older
> architecture) I don't care if the the dyn_release and dyn_cmp are in the
> same cache line as name.
> 
> -- Steve

This makes sense, I appreciate the explination! I was treating size_t as
an int in my head by accident.... oops!

-- Stevie