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