Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Speed up relocation process #344

Merged
merged 1 commit into from
Apr 10, 2022
Merged

Conversation

elicn
Copy link
Contributor

@elicn elicn commented Mar 14, 2022

Relocation can take up to several minutes for large files (e.g. 64-bit version of kernelbase.dll).

The reason is largely because __data__ stores contents as bytes, which does not have a __setitem__ method. When data needs to be modified it uses slicing - which casues the entire content to be copied over and over and slow down the entire relocation process. This patch simply turns __data__ into a bytearray and have it leverage its __setitem__ method to modify its contents much faster.

Note: after analyzing the slowdown rootcause and writing the patch, I noticed this issue was already reported and had a PR submitted to fix it almost 3 years ago (#266). I don't know why that PR was not accepted, but hopefully a fresh one would do.

Note: this patch does not maintain compatibility to Python2

@elicn
Copy link
Contributor Author

elicn commented Apr 7, 2022

Hi @erocarrera,
Is there anything specific that holds this PR from being merged..?
Please let me know if there is something I can do about it.

@erocarrera erocarrera merged commit e999b6d into erocarrera:master Apr 10, 2022
@erocarrera
Copy link
Owner

erocarrera commented Apr 10, 2022

Hi @elicn. Sorry for the delay, I don't get to work on pefile very regularly. The PR is great and the speed up very significant! I had overlooked #266, thanks for also bringing it to my attention.

@elicn
Copy link
Contributor Author

elicn commented Apr 10, 2022

Great, thanks!

@elicn elicn deleted the faster-reloc branch April 10, 2022 17:18
@psrok1
Copy link

psrok1 commented Apr 10, 2022

Thank you as well!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants