holzschu / ios_system

Drop-in replacement for system() in iOS programs
BSD 3-Clause "New" or "Revised" License
902 stars 147 forks source link

Break up into separate dylibs #13

Closed ian-mcdowell closed 6 years ago

ian-mcdowell commented 6 years ago

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 commented 6 years ago

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:

ian-mcdowell commented 6 years ago

To answer your questions:

holzschu commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

I think I can close this one.