Skip to content

Commit

Permalink
[GCU] Loading yang-models only once (sonic-net#1981)
Browse files Browse the repository at this point in the history
#### What I did
Loading sonic-yang models only once, and re-using them. This makes the sorting a lot faster.

How to verify `loadYangModel` took a lot of time?

Add the following snippet to `TestPatchSorter`
```python
from pstats import Stats
import cProfile
class TestPatchSorter(...):
    def setUp(self):
        """init each test"""
        self.pr = cProfile.Profile()
        self.pr.enable()
        print("\n<<<---")
    def tearDown(self):
        """finish any test"""
        p = Stats (self.pr)
        p.strip_dirs()
        p.sort_stats ('cumtime')
        p.print_stats ()
        print("\n--->>>")
    .
    .
    .
    # Also update verify(self, cc_ops=[], tc_ops=[]) by commenting out changes validation to avoid extra calls to loadYangModels 
    def verify(self, cc_ops=[], tc_ops=[]):
        # Arrange
        config_wrapper=ConfigWrapper()
        target_config=jsonpatch.JsonPatch(tc_ops).apply(Files.CROPPED_CONFIG_DB_AS_JSON)
        current_config=jsonpatch.JsonPatch(cc_ops).apply(Files.CROPPED_CONFIG_DB_AS_JSON)
        patch=jsonpatch.make_patch(current_config, target_config)

        # Act
        actual = self.create_patch_sorter(current_config).sort(patch)

        # Assert
        # simulated_config = current_config
        # for move in actual:
        #     simulated_config = move.apply(simulated_config)
        #     self.assertTrue(config_wrapper.validate_config_db_config(simulated_config))
        # self.assertEqual(target_config, simulated_config)

```
Run
```
> python3 -m unittest patch_sorter_test.TestPatchSorter.test_sort__dpb_1_to_4__success 
.
.
.
         48986582 function calls (48933431 primitive calls) in 104.530 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000  104.530  104.530 case.py:549(_callTestMethod)
        1    0.000    0.000  104.530  104.530 patch_sorter_test.py:1889(test_sort__dpb_1_to_4__success)
        1    0.000    0.000  104.529  104.529 patch_sorter_test.py:1933(verify)
        1    0.005    0.005  104.527  104.527 patch_sorter.py:1332(sort)
     32/1    0.006    0.000  104.077  104.077 patch_sorter.py:955(sort)
      334    0.012    0.000   99.498    0.298 patch_sorter.py:310(validate)
      492    2.140    0.004   95.810    0.195 sonic_yang_ext.py:30(loadYangModel)  <=========
```

From the above we can see profiling data about test_sort__dpb_1_to_4__success:
- Took 104.53s to complete
- loadYangModel was called 492 times
- loadYangModel took 95.810s.

loadYangModel is the method that loads the yang models from memory into SonicYang object. The loading of the YANG models should only happen once.

#### How I did it
Moved all calls to create sonic_yang object to ConfigWrapper, and each call to create a new instance just fills in the data for the yang models.

#### How to verify it
unit-test

Running profiling after the update:
```
> python3 -m unittest patch_sorter_test.TestPatchSorter.test_sort__dpb_1_to_4__success 
.
.
.
         702096 function calls (648951 primitive calls) in 2.882 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000    2.882    2.882 case.py:549(_callTestMethod)
        1    0.000    0.000    2.882    2.882 patch_sorter_test.py:1890(test_sort__dpb_1_to_4__success)
        1    0.002    0.002    2.881    2.881 patch_sorter_test.py:1934(verify)
        1    0.000    0.000    2.874    2.874 patch_sorter.py:1332(sort)
     32/1    0.004    0.000    2.705    2.705 patch_sorter.py:955(sort)
      334    0.008    0.000    2.242    0.007 patch_sorter.py:310(validate)
      490    0.080    0.000    1.791    0.004 sonic_yang_ext.py:1043(loadData)
      332    0.043    0.000    1.655    0.005 patch_sorter.py:345(validate)
      332    0.018    0.000    1.509    0.005 gu_common.py:112(validate_config_db_config)
        .
        .
        .
        1    0.002    0.002    0.164    0.164 sonic_yang_ext.py:30(loadYangModel)
```
From the above we can see profiling data about test_sort__dpb_1_to_4__success:
- Took 2.882s to complete
- loadYangModel was called 1time
- loadYangModel took 0.164s.

[profiling-after.txt](https://github.com/Azure/sonic-utilities/files/7757252/profiling-after.txt)

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
  • Loading branch information
ghooo committed Dec 22, 2021
1 parent bb56fc2 commit 3b642c9
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 22 deletions.
35 changes: 22 additions & 13 deletions generic_config_updater/gu_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def __eq__(self, other):
class ConfigWrapper:
def __init__(self, yang_dir = YANG_DIR):
self.yang_dir = YANG_DIR
self.sonic_yang_with_loaded_models = None

def get_config_db_as_json(self):
text = self._get_config_db_as_text()
Expand All @@ -63,8 +64,7 @@ def get_sonic_yang_as_json(self):
return self.convert_config_db_to_sonic_yang(config_db_json)

def convert_config_db_to_sonic_yang(self, config_db_as_json):
sy = sonic_yang.SonicYang(self.yang_dir)
sy.loadYangModel()
sy = self.create_sonic_yang_with_loaded_models()

# Crop config_db tables that do not have sonic yang models
cropped_config_db_as_json = self.crop_tables_without_yang(config_db_as_json)
Expand All @@ -76,8 +76,7 @@ def convert_config_db_to_sonic_yang(self, config_db_as_json):
return sonic_yang_as_json

def convert_sonic_yang_to_config_db(self, sonic_yang_as_json):
sy = sonic_yang.SonicYang(self.yang_dir)
sy.loadYangModel()
sy = self.create_sonic_yang_with_loaded_models()

# replace container of the format 'module:table' with just 'table'
new_sonic_yang_json = {}
Expand All @@ -100,8 +99,7 @@ def convert_sonic_yang_to_config_db(self, sonic_yang_as_json):
def validate_sonic_yang_config(self, sonic_yang_as_json):
config_db_as_json = self.convert_sonic_yang_to_config_db(sonic_yang_as_json)

sy = sonic_yang.SonicYang(self.yang_dir)
sy.loadYangModel()
sy = self.create_sonic_yang_with_loaded_models()

try:
sy.loadData(config_db_as_json)
Expand All @@ -112,8 +110,7 @@ def validate_sonic_yang_config(self, sonic_yang_as_json):
return False

def validate_config_db_config(self, config_db_as_json):
sy = sonic_yang.SonicYang(self.yang_dir)
sy.loadYangModel()
sy = self.create_sonic_yang_with_loaded_models()

try:
tmp_config_db_as_json = copy.deepcopy(config_db_as_json)
Expand All @@ -126,8 +123,7 @@ def validate_config_db_config(self, config_db_as_json):
return False

def crop_tables_without_yang(self, config_db_as_json):
sy = sonic_yang.SonicYang(self.yang_dir)
sy.loadYangModel()
sy = self.create_sonic_yang_with_loaded_models()

sy.jIn = copy.deepcopy(config_db_as_json)

Expand All @@ -151,6 +147,16 @@ def remove_empty_tables(self, config):
config_with_non_empty_tables[table] = copy.deepcopy(config[table])
return config_with_non_empty_tables

# TODO: move creating copies of sonic_yang with loaded models to sonic-yang-mgmt directly
def create_sonic_yang_with_loaded_models(self):
# sonic_yang_with_loaded_models will only be initialized once the first time this method is called
if self.sonic_yang_with_loaded_models is None:
loaded_models_sy = sonic_yang.SonicYang(self.yang_dir)
loaded_models_sy.loadYangModel() # This call takes a long time (100s of ms) because it reads files from disk
self.sonic_yang_with_loaded_models = loaded_models_sy

return copy.copy(self.sonic_yang_with_loaded_models)

class DryRunConfigWrapper(ConfigWrapper):
# This class will simulate all read/write operations to ConfigDB on a virtual storage unit.
def __init__(self, initial_imitated_config_db = None):
Expand All @@ -176,7 +182,7 @@ def _init_imitated_config_db_if_none(self):
class PatchWrapper:
def __init__(self, config_wrapper=None):
self.config_wrapper = config_wrapper if config_wrapper is not None else ConfigWrapper()
self.path_addressing = PathAddressing()
self.path_addressing = PathAddressing(self.config_wrapper)

def validate_config_db_patch_has_yang_models(self, patch):
config_db = {}
Expand Down Expand Up @@ -256,6 +262,10 @@ class PathAddressing:
"""
PATH_SEPARATOR = "/"
XPATH_SEPARATOR = "/"

def __init__(self, config_wrapper=None):
self.config_wrapper = config_wrapper

def get_path_tokens(self, path):
return JsonPointer(path).parts

Expand Down Expand Up @@ -391,8 +401,7 @@ def find_ref_paths(self, path, config):
return self._find_leafref_paths(path, config)

def _find_leafref_paths(self, path, config):
sy = sonic_yang.SonicYang(YANG_DIR)
sy.loadYangModel()
sy = self.config_wrapper.create_sonic_yang_with_loaded_models()

sy.loadData(config)

Expand Down
8 changes: 4 additions & 4 deletions generic_config_updater/patch_sorter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1119,21 +1119,21 @@ class IgnorePathsFromYangConfigSplitter:
def __init__(self, ignore_paths_from_yang_list, config_wrapper):
self.ignore_paths_from_yang_list = ignore_paths_from_yang_list
self.config_wrapper = config_wrapper
self.path_addressing = PathAddressing(config_wrapper)

def split_yang_non_yang_distinct_field_path(self, config):
config_with_yang = copy.deepcopy(config)
config_without_yang = {}

path_addressing = PathAddressing()
# ignore more config from config_with_yang
for path in self.ignore_paths_from_yang_list:
if not path_addressing.has_path(config_with_yang, path):
if not self.path_addressing.has_path(config_with_yang, path):
continue
if path == '': # whole config to be ignored
return {}, copy.deepcopy(config)

# Add to config_without_yang from config_with_yang
tokens = path_addressing.get_path_tokens(path)
tokens = self.path_addressing.get_path_tokens(path)
add_move = JsonMove(Diff(config_without_yang, config_with_yang), OperationType.ADD, tokens, tokens)
config_without_yang = add_move.apply(config_without_yang)

Expand Down Expand Up @@ -1325,7 +1325,7 @@ def __init__(self, config_wrapper, patch_wrapper, sort_algorithm_factory=None):
self.config_wrapper = config_wrapper
self.patch_wrapper = patch_wrapper
self.operation_wrapper = OperationWrapper()
self.path_addressing = PathAddressing()
self.path_addressing = PathAddressing(self.config_wrapper)
self.sort_algorithm_factory = sort_algorithm_factory if sort_algorithm_factory else \
SortAlgorithmFactory(self.operation_wrapper, config_wrapper, self.path_addressing)

Expand Down
45 changes: 44 additions & 1 deletion tests/generic_config_updater/gu_common_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,49 @@ def test_remove_empty_tables__multiple_empty_tables__returns_config_without_empt
# Assert
self.assertDictEqual({"any_table": {"key": "value"}}, actual)

def test_create_sonic_yang_with_loaded_models__creates_new_sonic_yang_every_call(self):
# check yang models fields are the same or None, non-yang model fields are different
def check(sy1, sy2):
# instances are different
self.assertNotEqual(sy1, sy2)

# yang models fields are same or None
self.assertTrue(sy1.confDbYangMap is sy2.confDbYangMap)
self.assertTrue(sy1.ctx is sy2.ctx)
self.assertTrue(sy1.DEBUG is sy2.DEBUG)
self.assertTrue(sy1.preProcessedYang is sy2.preProcessedYang)
self.assertTrue(sy1.SYSLOG_IDENTIFIER is sy2.SYSLOG_IDENTIFIER)
self.assertTrue(sy1.yang_dir is sy2.yang_dir)
self.assertTrue(sy1.yangFiles is sy2.yangFiles)
self.assertTrue(sy1.yJson is sy2.yJson)
self.assertTrue(not(hasattr(sy1, 'module')) or sy1.module is None) # module is unused, might get deleted
self.assertTrue(not(hasattr(sy2, 'module')) or sy2.module is None)

# non yang models fields are different
self.assertFalse(sy1.root is sy2.root)
self.assertFalse(sy1.jIn is sy2.jIn)
self.assertFalse(sy1.tablesWithOutYang is sy2.tablesWithOutYang)
self.assertFalse(sy1.xlateJson is sy2.xlateJson)
self.assertFalse(sy1.revXlateJson is sy2.revXlateJson)

config_wrapper = gu_common.ConfigWrapper()
self.assertTrue(config_wrapper.sonic_yang_with_loaded_models is None)

sy1 = config_wrapper.create_sonic_yang_with_loaded_models()
sy2 = config_wrapper.create_sonic_yang_with_loaded_models()

# Simulating loading non-yang model fields
sy1.loadData(Files.ANY_CONFIG_DB)
sy1.getData()

# Simulating loading non-yang model fields
sy2.loadData(Files.ANY_CONFIG_DB)
sy2.getData()

check(sy1, sy2)
check(sy1, config_wrapper.sonic_yang_with_loaded_models)
check(sy2, config_wrapper.sonic_yang_with_loaded_models)

class TestPatchWrapper(unittest.TestCase):
def setUp(self):
self.config_wrapper_mock = gu_common.ConfigWrapper()
Expand Down Expand Up @@ -443,7 +486,7 @@ def __assert_same_patch(self, config_db_patch, sonic_yang_patch, config_wrapper,

class TestPathAddressing(unittest.TestCase):
def setUp(self):
self.path_addressing = gu_common.PathAddressing()
self.path_addressing = gu_common.PathAddressing(gu_common.ConfigWrapper())
self.sy_only_models = sonic_yang.SonicYang(gu_common.YANG_DIR)
self.sy_only_models.loadYangModel()

Expand Down
9 changes: 5 additions & 4 deletions tests/generic_config_updater/patch_sorter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -921,8 +921,8 @@ def verify_diff(self, current_config, target_config, current_config_tokens=None,

class TestNoDependencyMoveValidator(unittest.TestCase):
def setUp(self):
path_addressing = ps.PathAddressing()
config_wrapper = ConfigWrapper()
path_addressing = ps.PathAddressing(config_wrapper)
self.validator = ps.NoDependencyMoveValidator(path_addressing, config_wrapper)

def test_validate__add_full_config_has_dependencies__failure(self):
Expand Down Expand Up @@ -1649,7 +1649,7 @@ def verify_moves(self, ex_ops, moves):

class DeleteRefsMoveExtender(unittest.TestCase):
def setUp(self):
self.extender = ps.DeleteRefsMoveExtender(PathAddressing())
self.extender = ps.DeleteRefsMoveExtender(PathAddressing(ConfigWrapper()))

def test_extend__non_delete_ops__no_extended_moves(self):
self.verify(OperationType.ADD,
Expand Down Expand Up @@ -1723,7 +1723,8 @@ def test_memoization_sorter(self):

def verify(self, algo, algo_class):
# Arrange
factory = ps.SortAlgorithmFactory(OperationWrapper(), ConfigWrapper(), PathAddressing())
config_wrapper = ConfigWrapper()
factory = ps.SortAlgorithmFactory(OperationWrapper(), config_wrapper, PathAddressing(config_wrapper))
expected_generators = [ps.LowLevelMoveGenerator]
expected_extenders = [ps.UpperLevelMoveExtender, ps.DeleteInsteadOfReplaceMoveExtender, ps.DeleteRefsMoveExtender]
expected_validator = [ps.DeleteWholeConfigMoveValidator,
Expand Down Expand Up @@ -1834,7 +1835,7 @@ def create_patch_sorter(self, config=None):
config_wrapper.get_config_db_as_json = MagicMock(return_value=config)
patch_wrapper = PatchWrapper(config_wrapper)
operation_wrapper = OperationWrapper()
path_addressing= ps.PathAddressing()
path_addressing= ps.PathAddressing(config_wrapper)
sort_algorithm_factory = ps.SortAlgorithmFactory(operation_wrapper, config_wrapper, path_addressing)

return ps.PatchSorter(config_wrapper, patch_wrapper, sort_algorithm_factory)
Expand Down

0 comments on commit 3b642c9

Please sign in to comment.