lasp / lasp/space_packet_parser
PERF: Remove builtin type subclassing
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 39
- Forks
- 15
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 12
Description
I was profiling some of our packet parsing and also looking into the Rust backend transition again.
There is a lot of performance time spent on creating our underlying data due to our use of __new__ on the builtin types.
This was added in v5, and followed the precedence of having the derived value and the raw value be "attached" to one another in the same class/struct, so that you could get one from the other. In v4, we had a ParsedDataItem that had all of this information associated with it.
https://github.com/lasp/space_packet_parser/blob/53d93736eefeb14cd157fd462046c0dabe6dc116/space_packet_parser/parser.py#L35
My proposal is to remove the direct link between raw value and a derived value and rather move that logic into the Packet container itself. So, we would only return built-in types from our parsing functions, not our subclassed types. Rather than returning common.FloatItem(parsed_value) we would return float(parsed_value). The Packet dictionary (Maybe our own MutableMapping instead...) would then actually be a two-dictionary storage structure pointing to a derived mapping and a raw mapping. (something like having the get try the derived version first and then fallback to the raw data if there was no derived value to avoid storing duplicates in each mapping)
class Packet(dict):
def __init__(self):
# separate storage location for the second container
self._derived_values = {}
def __get__(self, key):
# Try to return the derived value first, fallback to the raw if there was no derived version
return self._derived_values.get(key, super().get(key))
def get_raw(self, key):
return super().get(key)
The other place this is relevant is if we try and call spp_type + spp_type, we lose the raw_values anyways in the conversion because it doesn't really make sense which raw_value you'd want to keep between the types or both etc. It opens up a bit of a can of worms in terms of combining things together.
The path forward
We need to implement the new container with the new methods for getting raw/derived values, so users have something in place that they know they should use. Along with this, we can add a raw_value property to the project's builtin subclasses that warns whenever the raw_value attribute is accessed.
Then, in a following release, we change over to using builtins directly and remove our subclasses.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the builtin subclass implementation in common.py at lines 212-231 and the earlier ParsedDataItem design in parser.py at line 35. Implement the proposed container with raw and derived access methods, then add the raw_value deprecation warning to the existing builtin subclasses; done means users have the new access path before subclasses are removed in a later release.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, data, performance
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100