aminghadersohi commented on PR #42655:
URL: https://github.com/apache/superset/pull/42655#issuecomment-5201367604
Thanks — looked at this one, but leaving the code as-is. The premise doesn't
match what the helper does:
> serializes the entire entry on every `in`-set check ... O(n) overhead per
lookup
Each entry is serialized **exactly once**, not per lookup. The set is built
once up front, and each addition is keyed once; the `in` check is then an O(1)
hash lookup on an already-computed string:
```python
merged = list(existing)
seen = {_entry_key(entry) for entry in merged} # once per existing entry
for entry in additions:
key = _entry_key(entry) # once per addition
if key in seen: # O(1), no serialization
continue
```
So it's O(n+m) serializations total, not O(n) per lookup. Measured at 2× the
size cited (200 entries — 100 saved-metric names plus 100 adhoc dicts):
```
200 existing entries, 2000 calls -> 343.0 us per call
```
0.3 ms, once per `update_chart` call, alongside a dataset read, chart
compilation, and a network round-trip. Caching or a structural hash would add
state and complexity for no measurable gain, so I'd rather keep the
straightforward version.
Worth noting the serialization isn't incidental — it's what makes the dedup
correct. `metrics` mixes saved-metric names with adhoc dicts, so the obvious
`set()` raises `TypeError: unhashable type: 'dict'`, and it would also lose the
ordering that drives table column layout.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
