[ovs-dev,02/13] netdev-dummy: Reorder elements in dummy_packet_stream structure.

Message ID 1504893565-110166-3-git-send-email-bhanuprakash.bodireddy@intel.com
State Accepted
Headers show
Series
  • Rearrange structure members for memory efficiency.
Related show

Commit Message

Bhanuprakash Bodireddy Sept. 8, 2017, 5:59 p.m.
By reordering elements in dummy_packet_stream structure, sum holes
can be reduced, thus saving a cache line.

Before: structure size: 784, sum holes: 56, cachelines:13
After : structure size: 768, sum holes: 40, cachelines:12

Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
---
 lib/netdev-dummy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gregory Rose Sept. 15, 2017, 5:27 p.m. | #1
On 09/08/2017 10:59 AM, Bhanuprakash Bodireddy wrote:
> By reordering elements in dummy_packet_stream structure, sum holes

Do you mean "the sum of the holes" can be reduced or do you mean "some holes"
can be reduced?

Same question through several of the other patches where you use the same
language.

Thanks,

- Greg

> can be reduced, thus saving a cache line.
> 
> Before: structure size: 784, sum holes: 56, cachelines:13
> After : structure size: 768, sum holes: 40, cachelines:12
> 
> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
> ---
>   lib/netdev-dummy.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index f731af1..d888c40 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -50,8 +50,8 @@ struct reconnect;
>   
>   struct dummy_packet_stream {
>       struct stream *stream;
> -    struct dp_packet rxbuf;
>       struct ovs_list txq;
> +    struct dp_packet rxbuf;
>   };
>   
>   enum dummy_packet_conn_type {
>
Bhanuprakash Bodireddy Sept. 18, 2017, 6:49 a.m. | #2
Hi greg,

>On 09/08/2017 10:59 AM, Bhanuprakash Bodireddy wrote:
>> By reordering elements in dummy_packet_stream structure, sum holes
>
>Do you mean "the sum of the holes" can be reduced or do you mean "some
>holes"
>can be reduced?

In this patch series "sum of the holes" means, the sum/total of all the hole bytes in the
respective structure. For example 'dummy_packet_stream' structure members are aligned below way.
This structure has one hole comprising of 56 bytes.

struct dummy_packet_stream {
        struct stream *            stream;               /*     0     8 */
                                                                                                            ====>   56 bytes holes.                                                                                                                  
        struct dp_packet           rxbuf;               /*    64   704 */  
        struct ovs_list            txq;                        /*   768    16 */
};

With the proposed change in this patch, the new alignment is as below 

struct dummy_packet_stream {
        struct stream *            stream;               /*     0     8 */
        struct ovs_list            txq;                         /*     8    16 */
                                                                                                            ====> 40 bytes hole
        struct dp_packet           rxbuf;                /*    64   704 */
};

For all the patches, the information is added in to the commit log that shows
the improvement with the proposed changes. As claimed, sum holes(bytes) are
 reduced from 56 to 40 in case of this patch.

>> Before: structure size: 784, sum holes: 56, cachelines:13
>> After :  structure size: 768, sum holes: 40, cachelines:12

>
>Same question through several of the other patches where you use the same
>language.

In few structures there are multiple holes and 'sum holes' adds hole bytes of multiple holes
In those cases. 

- Bhanuprakash.

>
>> can be reduced, thus saving a cache line.
>>
>> Before: structure size: 784, sum holes: 56, cachelines:13 After :
>> structure size: 768, sum holes: 40, cachelines:12
>>
>> Signed-off-by: Bhanuprakash Bodireddy
>> <bhanuprakash.bodireddy@intel.com>
>> ---
>>   lib/netdev-dummy.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index
>> f731af1..d888c40 100644
>> --- a/lib/netdev-dummy.c
>> +++ b/lib/netdev-dummy.c
>> @@ -50,8 +50,8 @@ struct reconnect;
>>
>>   struct dummy_packet_stream {
>>       struct stream *stream;
>> -    struct dp_packet rxbuf;
>>       struct ovs_list txq;
>> +    struct dp_packet rxbuf;
>>   };
>>
>>   enum dummy_packet_conn_type {
>>
Ben Pfaff Nov. 3, 2017, 7:52 p.m. | #3
On Fri, Sep 08, 2017 at 06:59:14PM +0100, Bhanuprakash Bodireddy wrote:
> By reordering elements in dummy_packet_stream structure, sum holes
> can be reduced, thus saving a cache line.
> 
> Before: structure size: 784, sum holes: 56, cachelines:13
> After : structure size: 768, sum holes: 40, cachelines:12
> 
> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>

I wouldn't normally bother optimizing code that is used only for
testing, but the reduction in cache lines is nice, so I applied this.
Thank you!

Patch

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index f731af1..d888c40 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -50,8 +50,8 @@  struct reconnect;
 
 struct dummy_packet_stream {
     struct stream *stream;
-    struct dp_packet rxbuf;
     struct ovs_list txq;
+    struct dp_packet rxbuf;
 };
 
 enum dummy_packet_conn_type {