Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

autoPatchelfHook contains unused code intended for optimisation #148547

Closed
layus opened this issue Dec 4, 2021 · 4 comments · Fixed by #149731
Closed

autoPatchelfHook contains unused code intended for optimisation #148547

layus opened this issue Dec 4, 2021 · 4 comments · Fixed by #149731
Labels
0.kind: bug Something is broken

Comments

@layus
Copy link
Member

layus commented Dec 4, 2021

autoPatchelfHook has been modified to improve performance in #101142 but as far as I can tell, performance is still quadratic. Also, the current source code does not use the recently introduced associative array. It is only written to, hence the non increase in performance.

Pinging @DavHau could it be possible that your patch was not pushed in full, or only partial ? Looking at the current source code, does it look like the code you intended at the time ?

Pinging @aszlig for a second pair of eyes, and you feedback on the state of autoPatchelfHook.

Iff no-one works on this, I can submit a patch as we have abysmal performance on matlab which packages a tremendous amount of small shared objects.

@layus layus added the 0.kind: bug Something is broken label Dec 4, 2021
@layus layus changed the title autoPatchelfHook caontains unused code intended for optimisation autoPatchelfHook contains unused code intended for optimisation Dec 5, 2021
@layus
Copy link
Member Author

layus commented Dec 5, 2021

No kidding. ~16 hours for autoPatchelf to run 😅

[2021-12-04 14:32:55] automatically fixing dependencies for ELF files
...
[2021-12-05 06:12:03] autoPatchelfHook could not satisfy dependency libperl.so wanted by...
[2021-12-05 06:12:03] Add the missing dependencies to the build inputs or set ...

@DavHau
Copy link
Member

DavHau commented Feb 3, 2022

Sorry, didn't see this so far. The patch back then definitely increased the performance for me noticeable and it definitely fixes a quadratic issue whenever something is added to the cache. But that doesn't mean that the code is free of quadratic runtime issues. My PR possibly fixed only one of several issues.

@layus
Copy link
Member Author

layus commented Feb 6, 2022

@DavHau Thank you for your reply. It makes more sense now. My issue was with lookup time, so pretty orthogonal indeed.

@layus
Copy link
Member Author

layus commented Feb 6, 2022

I guess #149731 closes this issue 🎉. Thanks everyone for feedback and participation 🙏

@layus layus closed this as completed Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants