Repository navigation
Cache to_camel_case - #75
Conversation
DidlObject.__setattr__ resolves every attribute through to_camel_case, two re.sub calls each time, over a small closed set of property names. An LRU cache in front of it makes building objects ~5.8x faster and to_xml_string ~2.5x, with identical output. Bounded, since from_xml also passes names read from other devices' documents.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #75 +/- ##
==========================================
+ Coverage 94.77% 94.80% +0.02%
==========================================
Files 3 3
Lines 402 404 +2
Branches 45 45
==========================================
+ Hits 381 383 +2
Misses 12 12
Partials 9 9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thank you for the PR @cornopaez . Did you try any other values for the LRU cache size, other than 256? |
|
Thanks for looking at it. No, I hadn't: 256 was reasoned, not measured. I've measured it now, on the PR branch (Python 3.14, Apple M-series, median of five runs, ms). Three workloads: the benchmark from the description (build 10,000
Output hash and parsed item count were identical at every size. What decides it is the working set, not the number. Building and serialising touch 35 distinct names, and every size from 35 up is the same; at 34 the build is 70% slower, because an LRU one entry short of a cyclic working set evicts each name just before it is needed again. Parsing that one server's output touches 56 (the 35 plus Twonky's own fields, such as So any value comfortably above what a client will actually parse is equivalent, and a value just below it costs most of the gain. The working set grows with every vendor whose fields a control point reads, and one server already takes it to 56, so I'd rather keep headroom: 256 is about 4.5× that. The cost of headroom is small but not "a few kilobytes" as the description says. I measured a full 256-entry cache at about 37 KB ( I'm happy to change it to 128 if you prefer a tighter bound; it measured the same as 256 here. I'd not go below 64. And if you'd rather have Sweep scriptRun from a checkout of this branch. The argument is any file containing one or more import functools, hashlib, re, statistics, sys, time
from didl_lite import didl_lite, utils
RAW = utils.to_camel_case.__wrapped__
pages = re.findall(r"<DIDL-Lite.*?</DIDL-Lite>", open(sys.argv[1], encoding="utf-8").read(), re.S)
def build(n=10_000):
return [
didl_lite.MusicTrack(
id=f"0$all${i}", parent_id="0$all", restricted="1", title=f"Track {i}",
creator="Artist", artist="Artist", album="Album", genre="Classical",
original_track_number=str(i % 20 + 1), date="2020-01-01",
resources=[didl_lite.Resource(
uri=f"http://192.0.2.1:8200/res/{i}", protocol_info="http-get:*:audio/flac:*",
size="31457280", duration="0:05:00.000", bitrate="176400",
sample_frequency="96000", bits_per_sample="24", nr_audio_channels="2")],
)
for i in range(n)
]
def once(fn):
utils.to_camel_case = didl_lite.to_camel_case = fn
getattr(fn, "cache_clear", lambda: None)()
t0 = time.perf_counter(); items = build()
t1 = time.perf_counter(); xml = didl_lite.to_xml_string(*items)
t2 = time.perf_counter(); n = sum(len(didl_lite.from_xml_string(p, strict=False)) for p in pages)
t3 = time.perf_counter()
return t1 - t0, t2 - t1, t3 - t2, hashlib.sha256(xml).hexdigest()[:16], n
for size in [None, 256, 128, 64, 48, 35, 34, 16, "uncached"]:
fn = RAW if size == "uncached" else functools.lru_cache(maxsize=size)(RAW)
runs = [once(fn) for _ in range(5)]
med = [statistics.median(r[k] for r in runs) * 1000 for k in range(3)]
print(size, *(f"{m:.0f}" for m in med), {r[3] for r in runs}, runs[-1][4])AI disclosure: Claude Opus 5.5 (Anthropic) designed and ran the size sweep and the memory measurement on my machine and drafted this reply. I reviewed it and am responsible for it. |
|
256 is fine. Again, thank you for the PR. Do you want a release with this? |
|
A release would be preferred. Our code checks for this so it won't break, but I hope others may benefit from the enhancements as well. |
|
I've released 1.5.2. Given the loose version pinning in async-upnp-client ( |
DidlObject.__setattr__resolves every attribute throughdidl_property_key/didl_property_def_key, and both callutils.to_camel_case, which runs twore.subcalls each time. Its inputs are the property names in the class definitions (parentID,upnp:originalTrackNumber,@protocolInfo, …), a small closed set, so the same few dozen strings are converted again for every object and every attribute.This PR puts
functools.lru_cache(maxsize=256)onto_camel_case. Measured onmaster(c14a734) and on this branch, Python 3.14, Apple M-series, five runs each, with the benchmark below:masterMusicTrack(ten properties, oneResourceeach)to_xml_string(*items)a2ebda0ea28e1a1da2ebda0ea28e1a1dA counting run of the same workload makes 1,140,000 calls to
to_camel_caseover 35 distinct inputs (1,139,965 cache hits, 35 misses). Serialisation gains too, becauseto_xmllooks property definitions up through the same function.We found it in a DLNA media server that builds a whole container's children for a
Browse(13,390 tracks):cProfileput ~70% of the time in__setattr__→to_camel_case→re.sub, and SQLite at 2%. Caching it took that request from 6.8 s to 2.6 s on a Raspberry Pi 5; the response it builds is byte-identical with and without the cache (compared by hash).Change. The decorator on the definition in
utils.py. It covers both call sites:didl_lite.py'sfrom .utils import to_camel_casebinds the decorated function. (Patching the module attribute from outside does not, which is how we found there were two names.)Why bounded, not
maxsize=None.from_xml/from_xml_stringalso feed it tag and attribute names from documents received over the network. A long-running control point that parses DIDL from arbitrary devices should not grow a cache without limit. 256 is seven times the names in play; an LRU of that size costs a few kilobytes.Test. Existing tests already pin the output.
test_to_camel_case_is_cachedasserts that the function is cached, thatdidl_lite's imported name is the same cached function, and that building an object hits the cache, so a later refactor cannot drop it silently. It fails onmaster(hasattr(utils.to_camel_case, 'cache_info')is False). Happy to remove it if you would rather not test an implementation detail.Checks run locally from the pinned
requirements/tox-*sets: pytest (29 passed), ruff check and format, pylint (10.00/10), codespell, mypy, deptry, andpre-commit runon the changed files.Benchmark
Run once against each tree; the output hash shows the trees produce the same document.