Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[sonic-cfggen] optimize sonic-cfggen startup #3658

Merged
merged 4 commits into from
Oct 31, 2019

Conversation

stepanblyschak
Copy link
Collaborator

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Stepan Blyschak added 2 commits October 23, 2019 17:42
a lot of template rendering causes switch to start longer because jinja2
needs to parse them. Introducing RedisBytecodeCache to store parsed buckets of
internal template bytecode to speedup same template rendering during start

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
@lguohan
Copy link
Collaborator

lguohan commented Oct 23, 2019

do you have a benchmark number?

@lguohan lguohan requested a review from qiluo-msft October 24, 2019 06:57
@stepanblyschak
Copy link
Collaborator Author

@lguohan
cfggen_prof.txt
Test done with lazy_re. Note, sonic-cfggen is called over 100 times during start, this saves 8 sec for LAG restoration time. Ideally I think we could rethink the way start scripts work to eliminate many sonic-cfggen invocations.
Sonic-cfggen functionality could be implemented as a library that start scripts import only once (import jinja2, natsort are expensive because many static regular expressions are compiled at import time). This I believe will give a boost for start script like in bgp docker container https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-fpm-frr/start.sh where it renders many templates.
For scripts that require only hwsku, platform, hostname there could be a module that does not import heavy packages at all, since all you need is single redis query or read from machine.conf.
Some templates are questionable if they even have to be templates at all, like https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-dhcp-relay/wait_for_intf.sh.j2. A script could do this at runtime with few redis queries.
To speedup a bit templates rendering we can to use jinja cache in redis.
What do you think?

@@ -16,6 +16,16 @@ See usage string for detail description for arguments.
"""

from __future__ import print_function

# mokey patch re.compile to do lazy regular expression compilation.
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mokey [](start = 2, length = 5)

typo #Closed


re.compile = __re_compile


Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several code format issues. Please check http://pep8online.com #Closed

def __init__(self, client):
self._client = client
try:
self._client.connect(self._client.STATE_DB, retry_on=False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STATE_DB [](start = 46, length = 8)

use another non-used database?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I use db index 8 and update swss-common/swss-sdk?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still applicable.


In reply to: 338815799 [](ancestors = 338815799)

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Oct 24, 2019

do you have a benchmark for RedisBytecodeCache improvement? #Closed

@lguohan
Copy link
Collaborator

lguohan commented Oct 25, 2019

@stepanblyschak , i agree with all comments. good ideas!

@lguohan
Copy link
Collaborator

lguohan commented Oct 25, 2019

please address other reviewers comments.

Stepan Blyschak added 2 commits October 28, 2019 15:59
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
@stepanblyschak
Copy link
Collaborator Author

profiling_redis_bcc.txt

@lguohan lguohan merged commit 064689d into sonic-net:master Oct 31, 2019
@stepanblyschak stepanblyschak deleted the cfggen_opt branch September 23, 2022 13:31
mssonicbld added a commit that referenced this pull request Mar 2, 2025
…atically (#21343)

#### Why I did it
src/sonic-utilities
```
* 899ed9b9 - (HEAD -> master, origin/master, origin/HEAD) Remove Multi ASIC namespace Check. (#3783) (3 days ago) [Xincun Li]
* 9f1eab43 - Memory Statistics Config and Show Commands (#3575) (3 days ago) [Kanza Latif]
* 595c2aaa - Utilities Changes for DHCP DoS Mitigation Feature (#3301) (3 days ago) [Asad Raza]
* a3d15bc1 - Add multi-asic support for dropconfig (#3735) (3 days ago) [HP]
* 5ce06b27 - Add golden config check (#3770) (4 days ago) [Xincun Li]
* 3c50deeb - Fix `vnet_route_check` for active and inactive routes, add `--all` option (#3763) (9 days ago) [mramezani95]
* 40ba225f - Revert "enable pfcwd for backplane ports (#3759)" (#3767) (13 days ago) [Dashuai Zhang]
* 2866ccd9 - enable pfcwd for backplane ports (#3759) (2 weeks ago) [Dashuai Zhang]
* 3abd19e3 - [FC] remove FC delay field (#3577) (3 weeks ago) [Stepan Blyshchak]
* 11c27163 - Improve SONiC disk checker to handle disk full case and mount overlay fs to allow remote user login. (#3700) (3 weeks ago) [Hua Liu]
* 13619aa2 - [QOS] Skip showing unnecessary warning message (#3708) (3 weeks ago) [Vivek]
* f4e6e5b8 - Fixing 'show ip bgp neighbor <ip>' in frr unified config mode (#3738) (3 weeks ago) [kalash-nexthop]
* 9a18155e - Optimize lag_keepalive by crafting the LACPDU packet ourselves (#3170) (3 weeks ago) [Saikrishna Arcot]
* a7deb8c0 - display proper message with proper errno for kvm. (#3750) (3 weeks ago) [Dawei Huang]
* 865f196d - [Mellanox] Add new SN5640 platform and HwSKU (#3742) (4 weeks ago) [Noa Or]
* 5fa85027 - [show_techsupport][pstore] Archive /var/lib/systemd/pstore info to techsupport (#3745) (4 weeks ago) [Marty Y. Lok]
* 8f69d5a6 - sonic-utilities: WRED stats feature changes on sonic-utilities (#2807) (4 weeks ago) [Rajesh Perumal R]
* 9d273f16 - Fix call for spanning-tree commands in dump script (#3723) (4 weeks ago) [DavidZagury]
* 6d95d9bd - Make 'show ip bgp summary' work even when we don't have any peer groups (#3739) (4 weeks ago) [kalash-nexthop]
* 7f3957c4 - Fix ssdhealth failure on VS platform (#3743) (4 weeks ago) [Vivek]
* 97b4e4b8 - Fix show interface counters for Chassis Packet Supervisor (#3734) (4 weeks ago) [anamehra]
* 414935b7 - Add recover asic_id config if load golden config. (#3711) (5 weeks ago) [Xincun Li]
* 973cfdc3 - Remove partially installer image when image install failed. (#3712) (5 weeks ago) [Hua Liu]
* 97c20ccd - CLI support for SmartSwitch PMON (#3271) (5 weeks ago) [rameshraghupathy]
* 752c3d49 - [ACL] Display rule and table info written to APP DB (#3713) (5 weeks ago) [Vivek]
* fbd0c3b1 - [show][interface] Add changes for show interface flap command (#3724) (5 weeks ago) [vdahiya12]
* ddccabee - [show][interface] Add changes for show interface errors command (#3721) (5 weeks ago) [vdahiya12]
* be870a6e - [config] Exit with non-zero when qos reload fail (#3710) (6 weeks ago) [Jianyue Wu]
* ffa66e96 - Remove debug dump import by default (#3715) (6 weeks ago) [Vivek]
* fc3a3cbe - [ssdhealth] Check for default device before falling back to discovery (#3693) (6 weeks ago) [Vivek]
* 782c33a1 - [yang] Enforce yang full support in full config command (#3716) (6 weeks ago) [jingwenxie]
* a5b7a904 - [show][interfaces] Add proposal for show interfaces flap (#3627) (6 weeks ago) [vdahiya12]
* 0083a124 - sonic-utilities: add th5 hwskus to gcu conf file (#3714) (7 weeks ago) [Chris]
* 81cf04f7 - [Mellanox] Add Mellanox-SN5610N-C256S2, Mellanox-SN5610N-C224O8 to GCU validators (#3658) (7 weeks ago) [noaOrMlnx]
* 349a1018 - [db_migrator] Migrate tunnel table (#3704) (7 weeks ago) [Longxiang Lyu]
* 0e327c5f - show ip interfaces: fix exception with BGP unnumbered (#3695) (8 weeks ago) [Brad House]
* 7100f73b - [show_tech] modify generate_dump to includes BERT data and also the platform specified hw-mgmt info (#3676) (8 weeks ago) [Marty Y. Lok]
* 80d46988 - Delete TRSNCEIVER tables while config reload (#3680) (8 weeks ago) [noaOrMlnx]
* bf6ff9f9 - Enable multi asic golden config YANG validation. (#3685) (8 weeks ago) [Xincun Li]
```
#### How I did it
#### How to verify it
#### Description for the changelog
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants