swss: flush g_asicState after each event is done by dzhangalibaba · Pull Request #570 · sonic-net/sonic-swss

dzhangalibaba · 2018-08-09T21:43:48Z

add flush() after event is handled in case some entries are still in buffer, don't wait
with the changes in sairedis and swss-common, route performance improved by 200~300 routes/sec

Signed-off-by: Dong Zhang [email protected]

What I did

Why I did it

How I verified it

Details if related

* add flush() after event is handled in case some entries are still in buffer, don't wait * with the changes in sairedis and swss-common, route performance improved by 200~300 routes/sec Signed-off-by: Dong Zhang [email protected]

lguohan · 2018-08-10T07:33:03Z

you have a few PR submitted in sairedis, common, can you give dependencies on these PR? are they all independent or some pr requires other pr to be merge first?

dzhangalibaba · 2018-08-10T15:36:46Z

swss-common#218, this one should be merged first. sairedis#335 depends on swss-common#218. swss-common#218 and sairedis#335 are one feature.

The other three, swss#570 and sairedis#336 depends on swss-common#219, but they are independent in compiling point of view. So swss-common#219 should be merged first for these feature.

qiluo-msft · 2018-08-10T18:39:22Z

orchagent/orchdaemon.cpp

        for (Orch *o : m_orchList)
            o->doTask();

+        flush(); //flush after each event is handled, don't wait


//flush after e [](start = 17, length = 15)

Please help follow the coding style of other comments. #Closed

Sure. Will do

qiluo-msft · 2018-08-10T18:40:00Z

        flush();

So the flush() is not needed at all? #Closed

Refers to: orchagent/orchdaemon.cpp:302 in 8fa353c. [](commit_id = 8fa353c, deletion_comment = False)

qiluo-msft · 2018-08-10T18:42:20Z

orchagent/orchdaemon.cpp

        for (Orch *o : m_orchList)
            o->doTask();

+        flush(); //flush after each event is handled, don't wait


flush [](start = 8, length = 5)

About the performance "route performance improved by 200~300 routes/sec". Could you provide more details?

What is the test environment and test steps?

What is the performance before this PR and after?

If it is a small scope test, could you add a unit test or vs test case to automate it?
#Closed

the environment and test steps are listed on the sildes we discussed about two weeks ago . I already sent the slides that time internally, if you didn't get it, I can resent it to you.

The performance is listed in the slides as well. Only one PR didn't make sense, it involved many PRs for each optimization. We need to look at them together.

We tested the routing performance on physical switch , it matched what I listed on the slides.

OK. I just want to know performance before this PR and after? You only mentioned improved by 200~300 routes/sec. Rough numbers are ok. #Closed

before changes it is 1300 routes/sec, platform is brcm and SAI is 3.1, CPU is Intel(R) Atom(TM) CPU C2558 @ 2.40GHz. After this enabling pipeline changes only, it is about 1500-1600 routes/sec, if plus the syncd buffer changes in other PR, it is about 1700-1800 routes/sec

qiluo-msft

As comments

dzhangalibaba · 2018-08-10T22:25:18Z

Also , the flush() is needed, currently it will flush() when there is nothing to select, the SELECT_WAIT time is 1000ms, this is too long for routes, we need to flush them right away, don't wait the timeout

qiluo-msft · 2018-08-10T22:30:34Z

I mean the above flush() for timeout, since you already flush() after execute(). #Closed

dzhangalibaba · 2018-08-10T22:33:00Z

got it, then that flush is not necessary, we can remove it.

…mment *remove unnecessary flush() in timeout case and update comment Signed-off-by: Dong Zhang [email protected]

Unblock

**What I did** Revert #570 We should only `flush` the orchagent/syncd communication channel when there is no active tasks in orchagent. This will not influence the end-to-end performance in long run but introduce SELECT_TIMEOUT (1 s) latency if there is remaining data left inside the orchagent/syncd communication channel after previous `flush`, which is not a big deal. Fix sonic-net/sonic-buildimage#5570

sonic-net#1478) **What I did** Revert sonic-net#570 We should only `flush` the orchagent/syncd communication channel when there is no active tasks in orchagent. This will not influence the end-to-end performance in long run but introduce SELECT_TIMEOUT (1 s) latency if there is remaining data left inside the orchagent/syncd communication channel after previous `flush`, which is not a big deal. Fix sonic-net/sonic-buildimage#5570

* [syncd] Fix RPC compilation issues * Add missing rpc link for vssyncd

* swss: flush g_asicState after each event is done * add flush() after event is handled in case some entries are still in buffer, don't wait * with the changes in sairedis and swss-common, route performance improved by 200~300 routes/sec * swss-common: remove unnecessary flush() in timeout case and update comment * remove unnecessary flush() in timeout case and update comment

sonic-net#1478) **What I did** Revert sonic-net#570 We should only `flush` the orchagent/syncd communication channel when there is no active tasks in orchagent. This will not influence the end-to-end performance in long run but introduce SELECT_TIMEOUT (1 s) latency if there is remaining data left inside the orchagent/syncd communication channel after previous `flush`, which is not a big deal. Fix sonic-net/sonic-buildimage#5570

swss: flush g_asicState after each event is done

8fa353c

* add flush() after event is handled in case some entries are still in buffer, don't wait * with the changes in sairedis and swss-common, route performance improved by 200~300 routes/sec Signed-off-by: Dong Zhang [email protected]

lguohan requested a review from qiluo-msft August 10, 2018 07:32

qiluo-msft reviewed Aug 10, 2018

View reviewed changes

qiluo-msft previously requested changes Aug 10, 2018

View reviewed changes

dzhangalibaba added 2 commits August 10, 2018 15:39

swss-common: remove unnecessary flush() in timeout case and update co…

52dfe9f

…mment *remove unnecessary flush() in timeout case and update comment Signed-off-by: Dong Zhang [email protected]

swss-common: remove unnecessary flush() in timeout case and update co…

9531bcc

…mment *remove unnecessary flush() in timeout case and update comment Signed-off-by: Dong Zhang [email protected]

qiluo-msft approved these changes Aug 11, 2018

View reviewed changes

qiluo-msft merged commit fa7008f into sonic-net:master Aug 11, 2018

stcheng added the Enhancement ➕ label Aug 24, 2018

dzhangalibaba deleted the enable-g_asicState-pipeline branch September 27, 2019 23:07

qiluo-msft added a commit to qiluo-msft/sonic-swss that referenced this pull request Oct 23, 2020

Revert: swss: flush g_asicState after each event is done sonic-net#570

81f2599

qiluo-msft mentioned this pull request Oct 23, 2020

Revert: swss: flush g_asicState after each event is done #570 #1478

Merged

oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023

[syncd] Fix RPC compilation issues (sonic-net#570)

fe94170

* [syncd] Fix RPC compilation issues * Add missing rpc link for vssyncd

Conversation

dzhangalibaba commented Aug 9, 2018

Uh oh!

lguohan commented Aug 10, 2018

Uh oh!

dzhangalibaba commented Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhangalibaba Aug 10, 2018

Choose a reason for hiding this comment

Uh oh!

qiluo-msft commented Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhangalibaba Aug 10, 2018

Choose a reason for hiding this comment

Uh oh!

dzhangalibaba Aug 10, 2018

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhangalibaba Aug 11, 2018

Choose a reason for hiding this comment

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

dzhangalibaba commented Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft commented Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dzhangalibaba commented Aug 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dzhangalibaba commented Aug 10, 2018 •

edited

Loading

qiluo-msft Aug 10, 2018 •

edited

Loading

qiluo-msft commented Aug 10, 2018 •

edited

Loading

qiluo-msft Aug 10, 2018 •

edited

Loading

qiluo-msft Aug 10, 2018 •

edited

Loading

dzhangalibaba commented Aug 10, 2018 •

edited

Loading

qiluo-msft commented Aug 10, 2018 •

edited

Loading