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

Add build-time option to statically link to clap_entry #263

Merged
merged 2 commits into from
Jun 27, 2024
Merged

Add build-time option to statically link to clap_entry #263

merged 2 commits into from
Jun 27, 2024

Conversation

SamWindell
Copy link
Contributor

Allows for avoiding having to dlopen()/LoadLibrary() on itself to find
'clap_entry'. Instead it can be done at link-time.

Useful if you are linking the clap-wrapper files to your clap plugin.
This way, we avoid the potential point-of-failure and slowness of doing
library loading and filesystem access.

Let me know if I'm missing anything. And I might need assistance regarding how this integrates with the cmake. I don't the cmake system for my projects.

Allows for avoiding having to dlopen()/LoadLibrary() on itself to find
'clap_entry'. Instead it can be done at link-time.

Useful if you are linking the clap-wrapper files to your clap plugin.
This way, we avoid the potential point-of-failure and slowness of doing
library loading and filesystem access.
@defiantnerd defiantnerd changed the base branch from main to next June 21, 2024 18:43
@baconpaul
Copy link
Collaborator

@defiantnerd I mentioned on discord that this looks fine to me. Any objections? If not lets push it into next.

@defiantnerd
Copy link
Collaborator

defiantnerd commented Jun 27, 2024

I don't know, if this is really necessary - the dladdr() is referring to a compiled symbol address and won't make eny dlopen() (this is wrong in the documentation and needs to be changed) and the clap just exports the clap_entry entrypoint symbol to be found anyway, so there is no practical difference except the call to dladdr() and dlsym(), both against an already loaded library.

OTOH it adds another option to complexity.

I am not convinced.

@baconpaul
Copy link
Collaborator

So the primary reason is the clap entry as an exported symbol needs to be compiled into each DLL since you can’t have a DLL link another, if you want single file standalone

the way we solved this in conduit etc is we have the members of clap entry be statics and then link and multiply compile a file with the actual entry into each property

this aims to make it so you gave an alternate approach where clap entry is a static and you don’t need to export it at the edge

@defiantnerd
Copy link
Collaborator

ok - convinced.

@defiantnerd defiantnerd merged commit 497f40a into free-audio:next Jun 27, 2024
17 checks passed
@SamWindell
Copy link
Contributor Author

Thank you for the merge

I don't know, if this is really necessary - the dladdr() is referring to a compiled symbol address and won't make eny dlopen() (this is wrong in the documentation and needs to be changed) and the clap just exports the clap_entry entrypoint symbol to be found anyway, so there is no practical difference except the call to dladdr() and dlsym(), both against an already loaded library.

Does this code not dlopen based on the filename of the current module? Surely if it's loading based on filename its happening at runtime not compile/link time? I'm not trying to nitpick - there's probably a gap in my knowledge and I want know more!

@defiantnerd
Copy link
Collaborator

You can call dlopen() with the filename from dladdr() and it will return the handle from the already loaded image.

dlopen() returns a handle to be used in calls to dlclose(), dlsym(), and dlctl(). If the named shared object has already been loaded by a previous call to dlopen() and not yet unloaded by dlclose(), a handle referring to the resident copy is returned.

(see https://man.openbsd.org/dlopen.3)

So, as far as I understand it, the dlopen() call is not much overhead, when the name matches my own (determined by dladdr()).

@SamWindell
Copy link
Contributor Author

Ah interesting! Yes that makes sense then because the host would have had to dlopen us already so presumably us doing it again on ourselves should not cause another reload.

I still appreciate the static build option that was merged. I love removing (even unlikely) possible points of failure if the complexity cost isn't too high

@defiantnerd
Copy link
Collaborator

Yes, I haven't had the static linking to an actual executable in mind - this makes it necessary anyway.

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.

3 participants