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

Break up into separate dylibs #13

Closed
ian-mcdowell opened this issue Jan 29, 2018 · 8 comments
Closed

Break up into separate dylibs #13

ian-mcdowell opened this issue Jan 29, 2018 · 8 comments

Comments

@ian-mcdowell
Copy link
Contributor

I'd suggest building each command as a dynamic library (either .dylib or .framework), and lazy loading them at runtime.

As noted in the README, all the code for all of the commands is loaded into memory at app launch. This seems wasteful & unnecessary, and probably won't scale too well when adding more commands over time.

Additionally, it looks like they are currently being built into a single Xcode target. It'd be great if each command was its own target, with defined inputs/outputs (i.e. build these source files into this dylib). That way it's easier to pick & choose, and scales better.

@holzschu
Copy link
Owner

It's feasible ; I had it that way at some point in the history of the project. Even though dlclose() is a no-op, it would still decrease the application memory footprint.

The two questions I have (and where I need help) are:

  • would it impact chances of a project using the library being accepted in the AppStore? Does it count as a "separate API"?
  • circular dependencies: how to make a dylib that references ios_system() while being in a separate project? (python, lua, ed and others... can call system() or exec()).

@ian-mcdowell
Copy link
Contributor Author

To answer your questions:

  • No. This shouldn't have an affect on app review, as the symbols are still there. Not sure what you're referring to as a "separate API", however.
  • Circular dependencies are interesting. Assuming there are no circular dependencies between projects (not including ios_system), you should be able to structure as follows and be fine:
    • Make all dylib targets depend on ios_system target, so they can call system, exec, etc.
    • ios_system does not hard link to dylibs, but instead calls dlopen() when system() is called.

@holzschu
Copy link
Owner

You're right, that should work. I'm going to give it a try. @louisdh, as the sole official user of ios_system, would that impact your workflow? Any comments?

@louisdh
Copy link
Contributor

louisdh commented Jan 29, 2018

OpenTerm currently does import ios_system and calls ios_system(...) to execute a command. If those surface APIs stay the same, then OpenTerm shouldn't have much trouble adapting to any underlying structural changes.

@holzschu
Copy link
Owner

It's in the "dynamicLibraries" branch, if people want to test. I'm going to do some more testing before merging with main branch.

Pro: easier to compile the big projects (lua, python) who can be (almost) totally independent from iOS_system.
Pro: easier to disable commands. Just don't embed the corresponding library. So OpenTerm could use the same source as the main branch of ios_system, and simply not embed lua_ios.framework and python_ios.framework. For the user, the result is "command not found" if the program doesn't find the library (for now, there's a debug message about why the library didn't load, but I'll remove it before merging).
Con: a lot more libraries to move around. I didn't go with "one library for each command", more one for each package, but there's still 8 dynamic libraries to replace the single ios_system.framework. Plus lua_ios.framework and python_ios.framework if you want them.
Issue: a change of syntax for "replaceCommand" (replaced a pointer by a NSString).
Semi-issue: no memory gain when the libraries are dlclosed(). I think I saw a WWDC talk where it was said that dlclose() is currently a no-op. There's still a gain in memory, unless you use all the commands.

@holzschu
Copy link
Owner

The change broke share and open-url, because dlsym() needs the real name of the functions being called (and because it is in Swift and with objects isn't just "openUrl"). I'm looking into it.

@holzschu
Copy link
Owner

Aye, it works if I use the mangled name _T08OpenTerm7openUrls5Int32VAD4argc_SpySpys4Int8VGSgGSg4argvtF, but not with openUrl or OpenTerm.openUrl. It's not satisfying.
I'm open to suggestions, to either make Swift functions have C-callable names (the equivalent of extern "C") or to automatically mangle Swift function names.

@holzschu
Copy link
Owner

holzschu commented Feb 1, 2018

I think I can close this one.

@holzschu holzschu closed this as completed Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants