Skip to content

Commit 9aa198e

Browse files
BulkKeyGroupLowLevelMoveGenerator: optimize for restricted keys
Restricted keys (primarily PORT/*/admin_status) can't be grouped with other changes, but they themselves can be grouped together with eachother. This will speed up bulk changing of admin_status.
1 parent d6b0238 commit 9aa198e

1 file changed

Lines changed: 44 additions & 22 deletions

File tree

generic_config_updater/patch_sorter.py

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,11 @@ class BulkKeyGroupLowLevelMoveGenerator:
13531353
for a lot of change operations. We do still fall back to other more
13541354
primitive generators if the validators fail so this is an optimization that
13551355
otherwise doesn't have any impact on the overall outcome, only performance.
1356+
1357+
As a secondary optimization, restricted keys (primarily PORT/*/admin_status)
1358+
operations are grouped together as typically it is ok for all restricted keys
1359+
to change at the same time for the same table. If its not, the validator
1360+
will simply fail it and the generator will move on and "try" something else.
13561361
"""
13571362
def __init__(self, path_addressing):
13581363
self.generator = BulkLowLevelMoveGenerator(path_addressing)
@@ -1362,17 +1367,29 @@ def generate(self, diff):
13621367
for move in self.generate_groups(diff, self.generator.generate_remove):
13631368
yield move
13641369

1370+
# Handle removals for restricted keys in bulk independently
1371+
for move in self.generate_groups(diff, self.generator.generate_remove, restricted_only=True):
1372+
yield move
1373+
13651374
# Handle replacements next
13661375
for move in self.generate_groups(diff, self.generator.generate_replace):
13671376
yield move
13681377

1378+
# Handle replacements for restricted keys in bulk independently
1379+
for move in self.generate_groups(diff, self.generator.generate_replace, restricted_only=True):
1380+
yield move
1381+
13691382
# Handle additions last
13701383
for move in self.generate_groups(diff, self.generator.generate_add):
13711384
yield move
13721385

1373-
def generate_groups(self, diff, cb):
1386+
# Handle additions for restricted keys in bulk independently
1387+
for move in self.generate_groups(diff, self.generator.generate_add, restricted_only=True):
1388+
yield move
1389+
1390+
def generate_groups(self, diff, cb, restricted_only: bool=False):
13741391
group = None
1375-
for move in cb(diff, min_moves=1):
1392+
for move in cb(diff, min_moves=1, restricted_only=restricted_only):
13761393
if group is None:
13771394
group = move
13781395
continue
@@ -1419,40 +1436,44 @@ def generate(self, diff):
14191436
for move in self.generate_add(diff):
14201437
yield move
14211438

1422-
def generate_remove(self, diff, min_moves: int = 2):
1439+
def generate_remove(self, diff, min_moves: int = 2, restricted_only: bool = False):
14231440
self.diff = diff
14241441
tokens = []
14251442

14261443
for move in self.__traverse(OperationType.REMOVE, self.diff.current_config, self.diff.target_config, tokens,
1427-
min_moves):
1444+
min_moves, restricted_only):
14281445
yield move
14291446

1430-
def generate_replace(self, diff, min_moves: int = 2):
1447+
def generate_replace(self, diff, min_moves: int = 2, restricted_only: bool = False):
14311448
self.diff = diff
14321449
tokens = []
14331450

14341451
for move in self.__traverse(OperationType.REPLACE, self.diff.current_config, self.diff.target_config, tokens,
1435-
min_moves):
1452+
min_moves, restricted_only):
14361453
yield move
14371454

1438-
def generate_add(self, diff, min_moves: int = 2):
1455+
def generate_add(self, diff, min_moves: int = 2, restricted_only: bool = False):
14391456
self.diff = diff
14401457
tokens = []
14411458

14421459
for move in self.__traverse(OperationType.ADD, self.diff.current_config, self.diff.target_config, tokens,
1443-
min_moves):
1460+
min_moves, restricted_only):
14441461
yield move
14451462

1446-
def __restricted_key(self, tokens, key):
1463+
def __restricted_key(self, tokens, key, invert: bool=False):
14471464
tokens.append(key)
14481465
rv = self.requiredval.target_in_required_pattern(tokens)
14491466
tokens.pop()
1467+
if invert:
1468+
if rv:
1469+
return False
1470+
return True
14501471
return rv
14511472

1452-
def __traverse(self, op, current_ptr, target_ptr, tokens, min_moves):
1473+
def __traverse(self, op, current_ptr, target_ptr, tokens, min_moves, restricted_only: bool = False):
14531474
if isinstance(current_ptr, dict):
14541475
if self.__children_are_leafs(current_ptr) and self.__children_are_leafs(target_ptr):
1455-
for move in self.__output_bulk_move(op, current_ptr, target_ptr, tokens, min_moves):
1476+
for move in self.__output_bulk_move(op, current_ptr, target_ptr, tokens, min_moves, restricted_only):
14561477
yield move
14571478
return
14581479

@@ -1473,7 +1494,7 @@ def __traverse(self, op, current_ptr, target_ptr, tokens, min_moves):
14731494
continue
14741495

14751496
tokens.append(key)
1476-
for move in self.__traverse(op, current_ptr[key], target_ptr[key], tokens, min_moves):
1497+
for move in self.__traverse(op, current_ptr[key], target_ptr[key], tokens, min_moves, restricted_only):
14771498
yield move
14781499
tokens.pop()
14791500
return
@@ -1487,22 +1508,22 @@ def __children_are_leafs(self, config_ptr):
14871508
return False
14881509
return True
14891510

1490-
def __output_bulk_move(self, op, current_ptr, target_ptr, tokens, min_moves):
1511+
def __output_bulk_move(self, op, current_ptr, target_ptr, tokens, min_moves, restricted_only):
14911512
match op:
14921513
case OperationType.REMOVE:
1493-
for move in self.__output_bulk_remove(current_ptr, target_ptr, tokens, min_moves):
1514+
for move in self.__output_bulk_remove(current_ptr, target_ptr, tokens, min_moves, restricted_only):
14941515
yield move
14951516
case OperationType.REPLACE:
1496-
for move in self.__output_bulk_replace(current_ptr, target_ptr, tokens, min_moves):
1517+
for move in self.__output_bulk_replace(current_ptr, target_ptr, tokens, min_moves, restricted_only):
14971518
yield move
14981519
case OperationType.ADD:
1499-
for move in self.__output_bulk_add(current_ptr, target_ptr, tokens, min_moves):
1520+
for move in self.__output_bulk_add(current_ptr, target_ptr, tokens, min_moves, restricted_only):
15001521
yield move
15011522

1502-
def __output_bulk_add(self, current_ptr, target_ptr, tokens, min_moves):
1523+
def __output_bulk_add(self, current_ptr, target_ptr, tokens, min_moves, restricted_only):
15031524
group = JsonMoveGroup()
15041525
for key in target_ptr:
1505-
if current_ptr.get(key) is None and not self.__restricted_key(tokens, key):
1526+
if current_ptr.get(key) is None and not self.__restricted_key(tokens, key, invert=restricted_only):
15061527
tokens.append(key)
15071528
group.append(JsonMove(self.diff, OperationType.ADD, tokens, tokens))
15081529
tokens.pop()
@@ -1511,10 +1532,10 @@ def __output_bulk_add(self, current_ptr, target_ptr, tokens, min_moves):
15111532
if len(group) >= min_moves:
15121533
yield group
15131534

1514-
def __output_bulk_remove(self, current_ptr, target_ptr, tokens, min_moves):
1535+
def __output_bulk_remove(self, current_ptr, target_ptr, tokens, min_moves, restricted_only):
15151536
group = JsonMoveGroup()
15161537
for key in current_ptr:
1517-
if target_ptr.get(key) is None and not self.__restricted_key(tokens, key):
1538+
if target_ptr.get(key) is None and not self.__restricted_key(tokens, key, invert=restricted_only):
15181539
tokens.append(key)
15191540
group.append(JsonMove(self.diff, OperationType.REMOVE, tokens))
15201541
tokens.pop()
@@ -1523,11 +1544,12 @@ def __output_bulk_remove(self, current_ptr, target_ptr, tokens, min_moves):
15231544
if len(group) >= min_moves:
15241545
yield group
15251546

1526-
def __output_bulk_replace(self, current_ptr, target_ptr, tokens, min_moves):
1547+
def __output_bulk_replace(self, current_ptr, target_ptr, tokens, min_moves, restricted_only):
15271548
group = JsonMoveGroup()
15281549
for key in current_ptr:
15291550
target_val = target_ptr.get(key)
1530-
if target_val is not None and target_val != current_ptr.get(key) and not self.__restricted_key(tokens, key):
1551+
if (target_val is not None and target_val != current_ptr.get(key) and
1552+
not self.__restricted_key(tokens, key, invert=restricted_only)):
15311553
tokens.append(key)
15321554
group.append(JsonMove(self.diff, OperationType.REPLACE, tokens, tokens))
15331555
tokens.pop()

0 commit comments

Comments
 (0)