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

Use same objects for linking to shared and static libraries. #876

Closed
wants to merge 16 commits into from

Conversation

mmuetzel
Copy link
Contributor

Avoid building all objects twice (for the shared and the static library), effectively reducing the build time approximately by a factor of two.

Something similar could probably be done for most of the SuiteSparse libraries (including GraphBLAS). So, build time could probably be reduced by a lot (when building shared and static libraries).

@mmuetzel mmuetzel changed the title AMD: Use same object for linking to shared and static libraries. AMD: Use same objects for linking to shared and static libraries. Oct 18, 2024
@DrTimothyAldenDavis
Copy link
Owner

Looks great! That came out very clean.

My organization requires a minor revision to my Contributor Agreement, for all new contributions. Can you sign the new one I just posted? It's here in the dev branch:

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/dev/Contributor_License/SuiteSparse%20Individual%20Contributor%20License%20Agreement%20(20241011).pdf

Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
@mmuetzel mmuetzel changed the title AMD: Use same objects for linking to shared and static libraries. Use same objects for linking to shared and static libraries. Nov 13, 2024
@mmuetzel
Copy link
Contributor Author

I applied the same idea to some other SuiteSparse libraries and pushed here.

It looks like that doesn't work in general though. 😓
Maybe, position-independent code in static vs. shared libraries? On some platforms only? 🤔

@mmuetzel mmuetzel marked this pull request as draft November 13, 2024 19:12
… libraries.

Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
@mmuetzel
Copy link
Contributor Author

mmuetzel commented Nov 13, 2024

Explicitly compiling with -fPIC seems to fix it. (At least, it is able to link libsuitesparseconfig.so on Linux with that change.)

Would that be an ok fix? IIUC, static libraries are built without -fPIC by default...

Edit: I wonder why that wasn't necessary for libamd.so...

Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
Avoid building all objects twice (for the shared and the static library),
effectively reducing the build time approximately by a factor of two.
@DrTimothyAldenDavis
Copy link
Owner

Explicitly compiling with -fPIC seems to fix it. (At least, it is able to link libsuitesparseconfig.so on Linux with that change.)

Would that be an ok fix? IIUC, static libraries are built without -fPIC by default...

Edit: I wonder why that wasn't necessary for libamd.so...

Oh ... that could be a show-stopper. I think I shouldn't create static libraries with -fPIC. That could really be backward breaking change to the many users of the SuiteSparse static libraries. There's no telling what could break. It seems very dangerous to go down this path.

Adding -fPIC to static libraries can make them slower, for example. I think there's a reason that some people want to use static libraries and avoid -fPIC. The reasons might be more than just performance.

@mmuetzel mmuetzel closed this Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants