Skip to content

Rule efficiency improvements #62

Description

@Monarda

Intro

These linked issues were raised by Claude and text below is largely its.

is_applicable is evaluated twice per array element, and valueAlarm limits are re-read per element

Profiling ValueAlarmRule on an NTScalarArray post turned up two sources of repeated work in the shared rules machinery. Both are real, both are outside any single rule, and both are fixed in ScalarToArrayWrapperRule / the check_applicable_* decorators rather than in the rule that exposed them.

1. Double applicability check — ~46% of a post

ScalarToArrayWrapperRule._apply_elementwise (p4pillon/rules/rules.py:481-501) calls the wrapped rule once per element:

rule_flow = self._wrapped.post_rule(current_state, scalared_new_state)

BaseRule.post_rule is wrapped in @check_applicable_post, which calls self.is_applicable(newpvstate) (rules.py:91). The default post_rule body then delegates to init_rule (rules.py:282), which is wrapped in @check_applicable_init and calls is_applicable again (rules.py:74). So every element pays for two full applicability checks.

is_applicable (rules.py:242-259) is not cheap: a set(...).issubset(newpvstate.keys()) plus a Value.changed() call per declared field, all crossing into p4p.

Measured: stubbing the second check took a representative post from 17.05 ms to 9.22 ms — the single largest win available.

Worth noting that the answer cannot change between the two calls. The wrapper hoists a single scalared_new_state out of the loop and only reassigns ["value"], so the changed-set is fixed for the whole traversal. The check is not just duplicated per element, it is duplicated per element for a value that is constant across the entire array.

2. valueAlarm limits re-read per element — ~24%

ValueAlarmRule.__alarm_from_limits (p4pillon/rules/value_alarm_rule.py:135-138) reads eight valueAlarm.* fields out of the Value on every iteration:

for alarm_type, op in cls.LIMIT_CHECKS:
    severity = pvstate[f"valueAlarm.{alarm_type}Severity"]
    if severity and op(value, pvstate[f"valueAlarm.{alarm_type}Limit"]):

These are per-record constants — the same eight values for all N elements — but they are fetched from p4p (and the f-strings rebuilt) N times.

Measured: hoisting them out of the loop took the same post from 16.97 ms to 12.82 ms.

This is not fixable inside ValueAlarmRule alone without caching state on the rule instance, which rules are otherwise careful not to do. It needs a way for a gatherable rule to receive the array-constant portion of its inputs once per traversal rather than once per element.

Acceptance

  • Applicability is evaluated at most once per element per operation
  • Array-constant rule inputs are read once per traversal, not once per element
  • No change to any rule's observable behaviour; existing unit and integration suites pass
  • The # NOTE: Performance will be terrible! comment at rules.py:453 is updated or removed

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions