perf: load TestSet results as lightweight AttrDicts - #88
Open
mmghannam wants to merge 3 commits into
Open
Conversation
…uments Loading a TestSet's results dominated the comparison and evaluation views. Result documents are very wide (~1.7k fields each, one per IPET metric), and elasticsearch-dsl hydrating every field of every doc into Document/AttrDict objects is ~6x slower than reading the raw _source dicts (measured 1.14s vs 0.19s for a 328-instance run). The same results were also hydrated repeatedly: once per run in the comparison page, then again per run in the evaluation. load_results() now scans the raw _source dicts and wraps each in a small ResultHit(AttrDict) that carries meta.id as a real attribute (so it stays out of to_dict(), which feeds the IPET evaluation). All downstream code only needs attribute access, item access and to_dict(), which AttrDict already provides. delete_all_results() switches to a single delete-by-query since the loaded values are no longer Documents with .delete(). Output is byte-identical. Measured on six 328-instance runs: single run view 1.65s -> 0.41s comparison page 7.74s -> 1.50s evaluation 10.8s -> 2.70s Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPAST8EPQBUNTEB7ucBEXH
svigerske
reviewed
Aug 1, 2026
|
|
||
| def __init__(self, doc): | ||
| super().__init__(doc["_source"]) | ||
| self.meta = AttrDict({"id": doc["_id"]}) |
Member
There was a problem hiding this comment.
This doesn't seem to be executed or work and meta is None:
01-08-2026 10:35:58 - 359 ERROR tornado.application Uncaught exception GET /result/XiVLoZ8B-ExwBovct_Oy?compare=ayVEoZ8B-ExwBovccvAH (127.0.0.1)
HTTPServerRequest(protocol='http', host='rubberband.zib.de', method='GET', uri='/result/XiVLoZ8B-ExwBovct_Oy?compare=ayVEoZ8B-ExwBovccvAH', version='HTTP/1.1', remote_ip='127.0.0.1')
Traceback (most recent call last):
File "/usr/local/lib/python3.13/site-packages/tornado/web.py", line 1878, in _execute
result = method(*self.path_args, **self.path_kwargs)
File "/rubberband/rubberband/handlers/fe/result.py", line 89, in get
self.render(
~~~~~~~~~~~^
"result_view.html",
^^^^^^^^^^^^^^^^^^^
...<9 lines>...
evals=IPET_EVALUATIONS,
^^^^^^^^^^^^^^^^^^^^^^^
)
^
File "/usr/local/lib/python3.13/site-packages/tornado/web.py", line 1025, in render
html = self.render_string(template_name, **kwargs)
File "/usr/local/lib/python3.13/site-packages/tornado/web.py", line 1174, in render_string
return t.generate(**namespace)
~~~~~~~~~~^^^^^^^^^^^^^
File "/usr/local/lib/python3.13/site-packages/tornado/template.py", line 362, in generate
return execute()
File "result_view_html.generated.py", line 237, in _tt_execute
href="/instance/{rid}?compare={compare}".format(rid=r.meta.id, compare=','.join([c.meta.id for c in compare])) # results/details_table.html:32 (via results/res_view_details.html:37, result_view.html:65, wide.html:9, base.html:80)
^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'id'
01-08-2026 10:35:58 - 369 ERROR tornado.access 500 GET /result/XiVLoZ8B-ExwBovct_Oy?compare=ayVEoZ8B-ExwBovccvAH (127.0.0.1) 2599.97ms
Member
Author
There was a problem hiding this comment.
Thanks Stefan! seems that AttrDict.__setattr__ is overloaded and overrides the value of meta that is in the result. I changed it to just calling the default __setattr__ for any object directly.
…ults
ResultHit declared `meta = None` at class level so that AttrDict.__setattr__
would store the assigned meta as a real attribute. Its condition is
`name in self._d_ or not hasattr(self.__class__, name)`, so that only holds
while no result document has a field named "meta": when one does, the first
clause wins, meta is stored as a source field, and hit.meta then resolves to
the class attribute None. That 500'd the comparison view (details_table.html
calls r.meta.id), overwrote the document's own meta value, and leaked
{"id": ...} into to_dict(), which feeds the IPET evaluation. Setting meta with
object.__setattr__ bypasses the condition entirely.
get_data() also built its per-instance dicts straight from ResultHit.to_dict().
Unlike Document.to_dict(), AttrDict.to_dict() returns the underlying dict
rather than a copy, so the keys get_data() adds (instance_id, TimeLimit,
GitHash, RubberbandId, ...) were written into the cached results instead of a
throwaway. Copy before mutating.
Reported by @svigerske on scipopt#88.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkYsUSgc6FaPJxHtDUFzvW
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.
This avoids parsing each data entry from elastic search as a python object, and just keeps as it the data's original form: a dictionary.
Measured performance gain on six 328-instance runs:
single run view 1.65s -> 0.41s
comparison page 7.74s -> 1.50s
evaluation 10.8s -> 2.70s