XML reader: pool repeated attribute strings, and make attribute filters actually filter - #205
Merged
villelaitila merged 2 commits intoSep 11, 2026
Conversation
villelaitila
force-pushed
the
feature/pool-attribute-strings-on-parse
branch
from
September 11, 2026 14:49
fefbba1 to
424c56a
Compare
… occurrence expat hands out a fresh str object for every attribute name and value it reports, and sgraph models repeat both heavily. A 250k-element model carries 2.4M attribute occurrences drawn from 181 distinct names and ~90k distinct values, so its attribute names and values alone retained 2,831,679 string objects where 90,019 suffice. SGraphXMLParser now keeps a per-parse dict and routes attribute names, attribute values, element types and association dependency types through it, collapsing equal strings onto one object. Measured on three real models (retained RSS after parse, best of three runs): openclaw 627 MB -> 514 MB (-18%) intra 535 MB -> 429 MB (-20%) odoo-fullstack 473 MB -> 400 MB (-15%) Counted exactly rather than via RSS, the strings the openclaw model retains (element names, attribute names and values, dependency types) drop from 3,235,297 objects / 213.2 MB to 339,472 objects / 54.6 MB. Attribute-name objects alone go from 2,158,742 to 181. Parse time is unchanged within run-to-run noise. sys.intern() would collapse the same strings just as well - measured over this model it retains the identical 54.6 MB, and across two models loaded at once it shares only 0.2 MB more than this pool does. It is not used because it mutates interpreter-global state from a hot parsing loop for no measured gain, and because it raises TypeError on the None that an attribute written as <a n="x"/> legitimately produces. A dict on the parser instance keeps both the mechanism and the strings' lifetime local to the reader. The saving is a property of the data, not a guarantee: a synthetic model in which no name and no value repeats gains nothing and pays about 5% more RSS growth during the parse. Real models never look like that, because analyzers draw attribute names from a fixed vocabulary. Verified beyond the new tests: canonical dumps of every element path, its attributes, and every association's endpoints, deptype and attributes are identical to the pre-change reader on two 250k-element models, and to_deps output is byte-identical. Claude-Session: https://claude.ai/code/session_01XqfUmwZ3VYazHpKAFnYDFF
villelaitila
force-pushed
the
feature/pool-attribute-strings-on-parse
branch
from
September 11, 2026 14:50
424c56a to
fa49c34
Compare
elem_attribute_filters and assoc_attribute_filters were only partly wired up, in two independent ways. The <a> handler returned early only when BOTH ignore-all flags were set, so passing `IGNORE *` for one kind of attribute alone did nothing to the <a n=".." v=".."/> spelling. `assoc_attribute_filters=['IGNORE *']` kept every association attribute; `elem_attribute_filters=['IGNORE *']` kept every element attribute written as an <a> child. Separately, association attributes written inline on the <r> tag went through a loop that consulted no filter at all, so no assoc filter - ignore-all, blacklist or whitelist - ever reached that spelling. The equivalent loop for <e> already applied element filters; this brings <r> in line with it. Each kind of filter now governs its own kind of attribute, in both spellings. On a 250k-element model, `IGNORE *` on both filter lists now drops the 231,927 association attributes it previously kept, taking the loaded model from 465 MB to 426 MB. Structural values are deliberately left alone, as before: an element's type and an association's deptype describe what the thing is rather than data attached to it, and no attribute filter reaches them. No behaviour changes for a load that passes no filters - the canonical dump of two 250k-element models is identical to before. There were no tests for attribute filters at all, which is why this survived; tests/test_parse_attribute_filters.py now covers both filter kinds against both spellings, and 5 of its 10 tests fail without this change. Claude-Session: https://claude.ai/code/session_01XqfUmwZ3VYazHpKAFnYDFF
villelaitila
changed the title
Softagram Impact Report for pull/205 (head commit: 8a32290)TL;DR Arch. Impact: 📈 +4 | Changed code files: 3 | Directly impacted code files: 53⭐ Change Overview
⭐ Details of Dependency Changes (diagram)
🤖 AGENTS - machine-readable impact data (3 files changed, 53 impacted, +22/-0 deps)Change overviewHead Added dependencies (22)
Removed dependencies (0)None. Impacted files (53)Unchanged files that directly depend on files changed in this PR - check them for behavioral impact. Grouped by changed file; dependent paths starting with ./ are relative to the changed file's directory:
Complete data
[] 📄 Full report
Impact Report explained. Give feedback on this report to support@softagram.com |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits against the XML reader. The first is the memory change; the second is a filtering defect found while reading those same lines closely.
1. Pool repeated attribute strings
What
SGraphXMLParsernow keeps a per-parse string pool and routes attribute names, attribute values, element types and association dependency types through it, so equal strings collapse onto one object instead of one object per occurrence.Why
expat hands out a fresh
strfor every attribute name and value it reports, and sgraph models repeat both heavily. Measured on a 250k-element model: 2.4M attribute occurrences drawn from 181 distinct names and ~90k distinct values, yet attribute names and values alone retained 2,831,679 string objects where 90,019 suffice. Attribute-name objects accounted for 2,158,742 of those.Effect
Retained RSS after parse, three real models, best of three runs:
RSS is allocator-noisy, so the same thing counted exactly — every distinct string object the model retains, element names included:
Attribute-name objects: 2,158,742 → 181. Parse time is unchanged within run-to-run noise.
The saving is a property of the data, not a guarantee. A synthetic model in which no name and no value repeats gains nothing and pays about 5% more RSS growth during the parse for a pool it cannot use. Real models never look like that, because analyzers draw attribute names from a fixed vocabulary — so names pool even when values do not.
Why a per-parse dict rather than
sys.intern()sys.intern()collapses the same strings just as well: measured over this model it retains the identical 54.6 MB, and across two models held at once it shares only 0.2 MB more than this pool does. It also releases normally — runtime-interned strings are mortal on CPython 3.12/3.13/3.14, so this is not a leak argument.It is not used because it mutates interpreter-global state from a hot parsing loop for no measured gain, and because it raises
TypeErroron theNonethat an attribute written as<a n="x"/>legitimately produces. A dict on the parser instance keeps both the mechanism and the strings' lifetime local to the reader.Verification
tests/test_parse_string_pooling.py(11 tests), written before the change; 9 fail against unmodifiedmain. They assert object identity only as a proxy for "stored once" - the module docstring says so, because identity is not an sgraph guarantee and nothing in the library compares attribute strings withis. The two that pass onmainare invariant guards (no process-global interning;<a n="x"/>with novstill yieldsNone) — both were mutation-tested by swapping the pool forsys.intern, which breaks both. All six pooled call sites are covered, including thewhitelisted_elem_attributesbranch.spycyextra one cypher test skips - unrelated to this change.)to_depsoutput is byte-identical. (to_xmloutput differs run to run onmaintoo — pre-existing nondeterminism in id assignment, unrelated to this change.)flake8 --max-line-length=100: no new findings.Deliberately out of scope
parse_deps_linesbuilds attributes by string slicing and is not pooled. The deps format is documented as unsuitable for very large models, so the duplication there does not pay for the extra code.<r>, so pooling them is a separate change with its own risk.2. Make attribute filters reach every attribute they name
elem_attribute_filtersandassoc_attribute_filterswere only partly wired up, in two independent ways. Pre-existing, not introduced by the commit above — but that commit is what got these lines read closely.The
<a>handler returned early only when both ignore-all flags were set, so passingIGNORE *for one kind alone did nothing to the<a n=".." v=".."/>spelling.Association attributes written inline on
<r>consulted no filter at all, so no assoc filter — ignore-all, blacklist or whitelist — ever reached that spelling. The equivalent loop for<e>already applied element filters; this brings<r>in line with it.Measured on the same 250k-element model, asking for
IGNORE *on both lists:Structural values are deliberately left alone, as before: an element's
typeand an association'sdeptypedescribe what the thing is rather than data attached to it, and no attribute filter reaches them.test_structural_type_survives_every_filterpins that down so it reads as intent rather than as another gap.A load that passes no filters is unaffected — the canonical dump of two 250k-element models is identical to before, which is what almost every caller does.
There were no tests for attribute filters at all, which is why this survived.
tests/test_parse_attribute_filters.pynow covers both filter kinds against both spellings; 5 of its 10 tests fail against unmodifiedmain.https://claude.ai/code/session_01XqfUmwZ3VYazHpKAFnYDFF