On Fri, Mar 25, 2016 at 08:55:07AM +0800, Huang Lei wrote:
> From: Huang Lei <[email protected]>
>
> During our scalability test '2k HVs + 20k lports' we found that lexer is a
> major user of heap memory:
> - 5.22% ovn-controller libjemalloc.so.1 [.] free
> - free
> + 27.46% lexer_get
> + 18.00% ofctrl_put
> ...
> - 1.85% ovn-controller libjemalloc.so.1 [.] malloc
> - malloc
> - xmalloc
> - 55.03% xmemdup0
> - 90.58% lex_parse_id.isra.0
> - lexer_get
> ...
>
> So lex_token is modified to usage a 'buffer' defined in it for tokens smaller
> than 128 bytes, and for tokens bigger than 128 bytes it turn to use heap
> memory. This change makes our test case run at least 10% faster.
>
> Signed-off-by: Huang Lei <[email protected]>
This seems like a win, thank you for working on it. I have a few
suggestions.
First, I don't think that 'source' is necessary. It's sufficient to
test whether 's' points to the internal buffer. If it does, it's local
memory; otherwise, it's memory obtained from malloc().
Second, I don't think it's necessary to make lex_token larger. It's
already huge because it has two 128-byte mf_subvalue structures. We can
recycle that memory for the buffer.
Thus, I'd suggest something like this:
/* A token.
*
* 's' may point to 'buffer'; otherwise, it points to malloc()ed memory owned
* by the token. */
struct lex_token {
enum lex_type type; /* One of LEX_*. */
char *s; /* LEX_T_ID, LEX_T_STRING, LEX_T_ERROR only. */
enum lex_format format; /* LEX_T_INTEGER, LEX_T_MASKED_INTEGER only. */
union {
struct {
union mf_subvalue value; /* LEX_T_INTEGER, LEX_T_MASKED_INTEGER. */
union mf_subvalue mask; /* LEX_T_MASKED_INTEGER only. */
};
char buffer[256]; /* Buffer for LEX_T_ID/LEX_T_STRING. */
};
};
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
