diff --git a/src/sgraph/sgraph.py b/src/sgraph/sgraph.py
index 443053a..e5134f8 100644
--- a/src/sgraph/sgraph.py
+++ b/src/sgraph/sgraph.py
@@ -646,6 +646,38 @@ def __init__(self):
self.blacklisted_assoc_attributes: set[str] = set()
self.ignore_all_assoc_attributes = False
+ # Attribute names, attribute values, element types and dependency types
+ # repeat heavily across a model, but expat hands out a fresh str object per
+ # occurrence. A measured 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.83M string objects where 90k
+ # suffice. Pooling collapses those onto one object each: ~80% off the
+ # model's attribute-string bytes and 15-20% off its total footprint, at a
+ # parse cost inside 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 ~5% more RSS growth during the parse for a pool it
+ # cannot use. Real models never look like that - the analyzers draw
+ # attribute names from a fixed vocabulary - so names pool even when values
+ # do not.
+ # 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 legitimately produces. A dict on the
+ # parser instance keeps both the mechanism and the strings' lifetime local
+ # to the reader: the pool is discarded when parsing finishes, leaving the
+ # pooled strings reachable only through the model itself.
+ self._string_pool: dict[str | None, str | None] = {}
+
+ def _shared(self, value: str | None) -> str | None:
+ """Return the pool's canonical object for an equal string.
+
+ An attribute written as carries no value at all, so None
+ reaches this too and passes straight through, as it did before pooling.
+ """
+ return self._string_pool.setdefault(value, value)
+
def set_type_rules(self, the_type_rules: Optional[list[str]]):
if the_type_rules is None:
self.acceptableAssocTypes = None
@@ -706,6 +738,8 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
name = attrs.get('n')
if self.currentRelation is not None:
+ if self.ignore_all_assoc_attributes:
+ return
if name in self.blacklisted_assoc_attributes:
return
if self.whitelisted_assoc_attributes:
@@ -713,9 +747,11 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
return
value = attrs.get('v')
- self.currentRelation[name] = value # type: ignore
+ self.currentRelation[self._shared(name)] = self._shared(value)
else:
if self.currentElement is not None and len(self.currentElementPath) > 0:
+ if self.ignore_all_elem_attributes:
+ return
if name in self.blacklisted_elem_attributes:
return
if self.whitelisted_elem_attributes:
@@ -724,7 +760,8 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
self.property += 1
value = attrs.get('v')
- self.currentElement.addAttribute(name, value) # type: ignore
+ self.currentElement.addAttribute(self._shared(name),
+ self._shared(value))
else:
val = attrs.get('v')
sys.stderr.write(f' discarding {name} {val} attrs, no element to assign the data\n')
@@ -745,7 +782,7 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
for aname, avalue in list(attrs.items()):
if aname == 't' or aname == 'type':
- e.setType(avalue)
+ e.setType(self._shared(avalue))
self.property += 1
elif aname == 'i':
self.id_to_elem_map[avalue] = e
@@ -754,9 +791,11 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
if not aname in self.blacklisted_elem_attributes:
if self.whitelisted_elem_attributes:
if aname in self.whitelisted_elem_attributes:
- e.addAttribute(aname, avalue)
+ e.addAttribute(self._shared(aname),
+ self._shared(avalue))
else:
- e.addAttribute(aname, avalue)
+ e.addAttribute(self._shared(aname),
+ self._shared(avalue))
if self.only_root:
@@ -766,6 +805,8 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
self.currentRelation = {}
referred = attrs.get('r')
t = attrs.get('t')
+ if t is not None:
+ t = self._shared(t)
redirectEnabled = False
if not redirectEnabled:
self.link += 1
@@ -777,9 +818,20 @@ def startElement(self, tag_name: str, attrs: AttributesImpl):
elif referred is not None:
self.createReference(referred, t)
+ # 'r' and 't' carry the reference and the dependency type, not user
+ # attributes, and are excluded by the length test. The rest are
+ # association attributes and obey the same filters as children do.
for aname, avalue in list(attrs.items()):
if len(aname) > 1:
- self.currentRelation[aname] = avalue
+ if not self.ignore_all_assoc_attributes:
+ if aname not in self.blacklisted_assoc_attributes:
+ if self.whitelisted_assoc_attributes:
+ if aname in self.whitelisted_assoc_attributes:
+ self.currentRelation[self._shared(aname)] = \
+ self._shared(avalue)
+ else:
+ self.currentRelation[self._shared(aname)] = \
+ self._shared(avalue)
def endElement(self, name: str):
if name == 'e':
diff --git a/tests/test_parse_attribute_filters.py b/tests/test_parse_attribute_filters.py
new file mode 100644
index 0000000..3fbbedc
--- /dev/null
+++ b/tests/test_parse_attribute_filters.py
@@ -0,0 +1,106 @@
+"""elem_attribute_filters and assoc_attribute_filters must each govern their own
+kind of attribute, independently, and must reach both spellings the XML format
+allows: an child, and an attribute written inline on the
+enclosing or tag."""
+import io
+
+from sgraph import SGraph
+
+# Every element and every association carries the same two attributes, once in
+# each spelling, so a filter that reaches only one spelling is visible.
+MODEL = """
+
+
+
+
+
+
+
+
+
+
+
+
+"""
+
+ALL_ELEM = {'elem_inline': 'ei', 'elem_child': 'ec'}
+ALL_ASSOC = {'assoc_inline': 'ai', 'assoc_child': 'ac'}
+
+
+def load(**kwargs):
+ """Element and association attributes of /repo/a.py, minus the structural ones.
+
+ 'type' on an element and deptype on an association describe what the thing is,
+ not data attached to it, so no attribute filter is meant to reach them - see
+ test_structural_type_survives_every_filter.
+ """
+ graph = SGraph.parse_xml_file_or_stream(io.StringIO(MODEL), **kwargs)
+ repo = graph.rootNode.children[0]
+ elem = next(c for c in repo.children if c.name == 'a.py')
+ elem_attrs = {k: v for k, v in elem.attrs.items() if k != 'type'}
+ return elem_attrs, dict(elem.outgoing[0].attrs or {})
+
+
+def test_without_filters_every_attribute_is_kept():
+ assert load() == (ALL_ELEM, ALL_ASSOC)
+
+
+def test_assoc_ignore_all_drops_association_attributes_in_both_spellings():
+ elem_attrs, assoc_attrs = load(assoc_attribute_filters=['IGNORE *'])
+
+ assert assoc_attrs == {}
+ assert elem_attrs == ALL_ELEM, 'association filters must not touch element attributes'
+
+
+def test_elem_ignore_all_drops_element_attributes_in_both_spellings():
+ elem_attrs, assoc_attrs = load(elem_attribute_filters=['IGNORE *'])
+
+ assert elem_attrs == {}
+ assert assoc_attrs == ALL_ASSOC, 'element filters must not touch association attributes'
+
+
+def test_ignoring_everything_drops_everything():
+ assert load(elem_attribute_filters=['IGNORE *'],
+ assoc_attribute_filters=['IGNORE *']) == ({}, {})
+
+
+def test_assoc_blacklist_reaches_both_spellings():
+ assert load(assoc_attribute_filters=['IGNORE assoc_inline'])[1] == {'assoc_child': 'ac'}
+ assert load(assoc_attribute_filters=['IGNORE assoc_child'])[1] == {'assoc_inline': 'ai'}
+
+
+def test_assoc_whitelist_reaches_both_spellings():
+ assert load(assoc_attribute_filters=['assoc_inline'])[1] == {'assoc_inline': 'ai'}
+ assert load(assoc_attribute_filters=['assoc_child'])[1] == {'assoc_child': 'ac'}
+
+
+def test_elem_blacklist_reaches_both_spellings():
+ assert load(elem_attribute_filters=['IGNORE elem_inline'])[0] == {'elem_child': 'ec'}
+ assert load(elem_attribute_filters=['IGNORE elem_child'])[0] == {'elem_inline': 'ei'}
+
+
+def test_elem_whitelist_reaches_both_spellings():
+ assert load(elem_attribute_filters=['elem_inline'])[0] == {'elem_inline': 'ei'}
+ assert load(elem_attribute_filters=['elem_child'])[0] == {'elem_child': 'ec'}
+
+
+def test_structural_type_survives_every_filter():
+ """An element's type is set outside the attribute-filter path on purpose."""
+ graph = SGraph.parse_xml_file_or_stream(io.StringIO(MODEL),
+ elem_attribute_filters=['IGNORE *'],
+ assoc_attribute_filters=['IGNORE *'])
+ repo = graph.rootNode.children[0]
+ elem = next(c for c in repo.children if c.name == 'a.py')
+
+ assert elem.attrs == {'type': 'file'}
+
+
+def test_filtering_association_attributes_keeps_the_association_itself():
+ graph = SGraph.parse_xml_file_or_stream(io.StringIO(MODEL),
+ assoc_attribute_filters=['IGNORE *'])
+ repo = graph.rootNode.children[0]
+ elem = next(c for c in repo.children if c.name == 'a.py')
+
+ assert len(elem.outgoing) == 1
+ assert elem.outgoing[0].deptype == 'use'
+ assert elem.outgoing[0].toElement.name == 'b.py'
diff --git a/tests/test_parse_string_pooling.py b/tests/test_parse_string_pooling.py
new file mode 100644
index 0000000..b07dddb
--- /dev/null
+++ b/tests/test_parse_string_pooling.py
@@ -0,0 +1,160 @@
+"""The XML reader must hand out one string object per distinct attribute name,
+attribute value, element type and dependency type, instead of a fresh object per
+occurrence. Models repeat these heavily, so the duplicates dominate the loaded
+model's string footprint.
+
+These tests assert object identity because it is the only cheap proxy for "this
+string is stored once". Identity is not an sgraph guarantee: nothing in the
+library compares attribute strings with `is`, and nothing may start to. Compare
+attribute values with `==`."""
+import io
+
+from sgraph import SGraph
+
+# Two sibling files carrying the same attribute names and the same values, plus a
+# dependency between them. Values are multi-character so that CPython's own
+# caching of latin-1 singletons cannot be mistaken for pooling.
+MODEL = """
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+"""
+
+
+# Same shape, but the association attributes are written as XML attributes on .
+INLINE_ASSOC_MODEL = """
+
+
+
+
+
+
+
+
+
+
+"""
+
+
+def parse():
+ return SGraph.parse_xml_file_or_stream(io.StringIO(MODEL))
+
+
+def files_of(graph):
+ repo = graph.rootNode.children[0]
+ return {child.name: child for child in repo.children}
+
+
+def test_repeated_attribute_names_share_one_object():
+ files = files_of(parse())
+ name_a = next(k for k in files['a.py'].attrs if k == 'analyzer_name')
+ name_b = next(k for k in files['b.py'].attrs if k == 'analyzer_name')
+
+ assert name_a is name_b
+
+
+def test_repeated_attribute_values_share_one_object():
+ files = files_of(parse())
+
+ assert files['a.py'].attrs['analyzer_name'] is files['b.py'].attrs['analyzer_name']
+
+
+def test_repeated_inline_attribute_values_share_one_object():
+ """Element attributes written as XML attributes on , not as children."""
+ files = files_of(parse())
+
+ assert files['a.py'].attrs['language'] is files['b.py'].attrs['language']
+
+
+def test_repeated_inline_attribute_names_share_one_object():
+ """expat does not reuse attribute-name objects across a document, so the inline
+ form needs pooling just as much as the form does."""
+ files = files_of(parse())
+ name_a = next(k for k in files['a.py'].attrs if k == 'language')
+ name_b = next(k for k in files['b.py'].attrs if k == 'language')
+
+ assert name_a is name_b
+
+
+def test_repeated_inline_association_attribute_names_share_one_object():
+ """Association attributes written as XML attributes on ."""
+ graph = SGraph.parse_xml_file_or_stream(io.StringIO(INLINE_ASSOC_MODEL))
+ files = files_of(graph)
+ attrs_a = files['a.py'].outgoing[0].attrs
+ attrs_b = files['b.py'].outgoing[0].attrs
+
+ name_a = next(k for k in attrs_a if k == 'call_context')
+ name_b = next(k for k in attrs_b if k == 'call_context')
+ assert name_a is name_b
+ assert attrs_a['call_context'] is attrs_b['call_context']
+
+
+def test_repeated_element_types_share_one_object():
+ files = files_of(parse())
+
+ assert files['a.py'].typeEquals(files['b.py'].getType())
+ assert files['a.py'].attrs['type'] is files['b.py'].attrs['type']
+
+
+def test_repeated_dependency_types_share_one_object():
+ files = files_of(parse())
+ dep_a = files['a.py'].outgoing[0]
+ dep_b = files['b.py'].outgoing[0]
+
+ assert dep_a.deptype == 'function_ref'
+ assert dep_a.deptype is dep_b.deptype
+
+
+def test_repeated_association_attributes_share_one_object():
+ files = files_of(parse())
+ attrs_a = files['a.py'].outgoing[0].attrs
+ attrs_b = files['b.py'].outgoing[0].attrs
+
+ name_a = next(k for k in attrs_a if k == 'call_context')
+ name_b = next(k for k in attrs_b if k == 'call_context')
+ assert name_a is name_b
+ assert attrs_a['call_context'] is attrs_b['call_context']
+
+
+def test_whitelisted_inline_attributes_are_pooled():
+ """The whitelist branch of the inline-element-attribute path pools too."""
+ graph = SGraph.parse_xml_file_or_stream(io.StringIO(MODEL),
+ elem_attribute_filters=['language'])
+ files = files_of(graph)
+
+ assert 'language' in files['a.py'].attrs
+ name_a = next(k for k in files['a.py'].attrs if k == 'language')
+ name_b = next(k for k in files['b.py'].attrs if k == 'language')
+ assert name_a is name_b
+ assert files['a.py'].attrs['language'] is files['b.py'].attrs['language']
+
+
+def test_pooling_does_not_outlive_the_parse():
+ """The pool must not be process-global: two parses produce independent objects,
+ so a long-lived process that loads many models can free each model's strings."""
+ first = files_of(parse())['a.py'].attrs['analyzer_name']
+ second = files_of(parse())['a.py'].attrs['analyzer_name']
+
+ assert first == second
+ assert first is not second
+
+
+def test_attribute_without_value_still_parses_as_none():
+ xml = ''
+ graph = SGraph.parse_xml_file_or_stream(io.StringIO(xml))
+ elem = graph.rootNode.children[0].children[0]
+
+ assert elem.attrs['novalue'] is None