> 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.
Jarno
> Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
