java-topology/docs/tickets/pylons-0003-toposorter-order-list-remove.md

1.6 KiB

pylons-0003: TopologicalSorter.remove() — O(N*E) self.order.remove() inside edge loop

Severity: MEDIUM File: src/pyramid/util.py (Pyramid web framework, Pylons project) Lines: 455, 460 Status: PATCHED

Description

TopologicalSorter.remove() deletes a node and its edges from self.order, which is a plain list of (a, b) tuples. For each edge (u, name) it calls self.order.remove() — an O(E) list scan:

def remove(self, name):
    self.names.remove(name)                # O(N) scan
    ...
    for u in after:
        self.order.remove((u, name))       # O(E) scan — CWE-407
    ...
    for u in before:
        self.order.remove((name, u))       # O(E) scan — CWE-407

remove() is called from add() (line 482) whenever a name is re-added — every duplicate tween/deriver registration triggers this path. With D duplicates each having K before/after constraints: O(D * K * E) total.

Root Cause

self.order is an unindexed list. Removal requires a linear scan to find the tuple. A dict or set of tuples gives O(1) discard.

Fix

Convert self.order to a set (edges are unique pairs):

self.order = set()   # was: []
# add: self.order.add((u, name))   / self.order.add((name, o))
# remove: self.order.discard((u, name))

sorted() iterates self.order — iteration over a set is still O(E), correct.

See patch: defects/pylons/patch/pylons-0003-toposorter-order-set.patch

Speedup

D=100 re-registrations, K=3 constraints, E=300 edges:

  • Slow: 100 * 3 * 300 = 90,000 tuple comparisons
  • Fast: 100 * 3 * 1 = 300 hash lookups
  • Speedup: ~300x; grows with E