On Wed, Jul 15, 2015 at 01:11:54PM -0700, Jarno Rajahalme wrote:
>
> > On Jul 14, 2015, at 4:50 PM, Ben Pfaff <b...@nicira.com> wrote:
> >
> > On Fri, Jul 10, 2015 at 10:35:21AM -0700, Jarno Rajahalme wrote:
> >> Now that performance critical code already inlines miniflows and
> >> minimasks, we can simplify struct miniflow by always dynamically
> >> allocating miniflows and minimasks to the correct size. This changes
> >> the struct minimatch to always contain pointers to its miniflow and
> >> minimask.
> >>
> >> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
> >
> > I like simplifying things so that two possibilities become one. It's
> > nice.
> >
> > I see two possible ways to simplify this, each with its own tradeoffs:
> >
> > 1. The way that you did it, which means that one must always
> > either dynamically allocate struct miniflow to a proper size
> > or ensure that there's enough space for the maximum size flow
> > immediately following struct miniflow.
> >
> > 2. Change struct miniflow to:
> >
> > struct miniflow {
> > uint64_t map;
> > uint64_t *values;
> > };
> >
> > With this strategy, there's no need to dynamically allocate
> > struct miniflow itself, so it can be directly embedded in
> > larger structures instead of via pointer. The usual time cost
> > of accessing miniflow data is again one pointer dereference
> > (of 'values').
> >
> > This can be kind of nice because you're not fighting with the
> > compiler to put data in the right place.
> >
> > Either way, the usual time cost of accessing miniflow data is one
> > pointer dereference (in case #1, from whatever contains the miniflow; in
> > case #2, dereference of 'values'), and the usual space cost of embedding
> > a miniflow is one pointer (although in case #1 the pointer is in what
> > contains the miniflow and in case #2 the pointer is in the miniflow
> > itself).
> >
> > Anyway, I wanted to make sure that you considered both of these
> > solutions and decided that #1 was better. If so:
> >
>
> I agree that case #2 is syntactically nicer in the generic case, where
> one pointer dereference is always needed. However, we already embed
> miniflows to other data structures (flow keys and masks in classifier
> and userspace datapath), where avoid the dereference and hence make
> accessing the miniflow faster. With case #2 we’d need to define
> another “struct inline_miniflow” (basically the case #1) to avoid the
> dereference. IMO it is better to use the case #1 everywhere than
> defining two different miniflow structures.
OK, I'm glad that you considered the possibility then.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
