Fix performance issues with large binary files #11
Conversation
Specifically, pack and unpack PropWords properties with list
comprehensions rather than appending each word to a bytes object
(creating a new object each time), or to an array.
Also use struct.Struct to avoid having to compile the format millions of
times.
The changes to writing out the dtb make a factor of at least x1000 when
writing out a 20Mb file ("at least" because I lost patience). The
changes to reading in are much less dramatic - but it's still at least
x3. I can read in and write out the 20Mb file in about 1.5s now.
Pull Request Test Coverage Report for Build 42
💛 - Coveralls |
| @@ -41,11 +42,10 @@ def new_property(name: str, raw_value: bytes) -> object: | |||
| elif len(raw_value) and len(raw_value) % 4 == 0: | |||
| obj = PropWords(name) | |||
| # Extract words from raw value | |||
| for i in range(0, len(raw_value), 4): | |||
| obj.append(unpack(">I", raw_value[i:i + 4])[0]) | |||
| obj.data = [BIGENDIAN_WORD.unpack(raw_value[i:i + 4])[0] for i in range(0, len(raw_value), 4)] | |||
There was a problem hiding this comment.
There are two changes here. Firstly, use BIGENDIAN_WORD.unpack so that we don't have to compile the format a few million times. If we use Struct at all, this cannot be controversial (it speeds up reading big files by x2).
Secondly, rather than use .append on what we know is an initially empty list, construct the list with a single comprehension and then assign it. It could be argued that this is breaking the encapsulation of PropWords - if so, I can write a mutator for PropWords which takes the list and assigns it (it's worth another x2 on performance).
| return obj | ||
|
|
||
| elif len(raw_value) and len(raw_value): |
There was a problem hiding this comment.
This is not a necessary part of the change - it just seemed very odd, so I cleaned it up.
| @@ -342,8 +342,7 @@ def to_dtb(self, strings: str, pos: int = 0, version: int = Header.MAX_VERSION): | |||
| strpos = len(strings) | |||
| strings += self.name + '\0' | |||
| blob = pack('>III', DTB_PROP, len(self.data) * 4, strpos) | |||
| for word in self.data: | |||
| blob += pack('>I', word) | |||
| blob += bytes().join([BIGENDIAN_WORD.pack(word) for word in self.data]) | |||
There was a problem hiding this comment.
This is the critical change - it makes the write operation complete in a small number of seconds rather than at least 10's of minutes.
Specifically, pack and unpack PropWords properties with list
comprehensions rather than appending each word to a bytes object
(creating a new object each time), or to an array.
Also use struct.Struct to avoid having to compile the format millions of
times.
The changes to writing out the dtb make a factor of at least x1000 when
writing out a 20Mb file ("at least" because I lost patience). The
changes to reading in are much less dramatic - but it's still at least
x3. I can read in and write out the 20Mb file in about 1.5s now.