[TIMOB-26015] Android/iOS: Refactored "app.js" launch handling #10112
Conversation
- Added a new "titanium_mobile/templates/core/Resources" JavaScript folder.
* Provides JS files/extensions intended to be shared by all platforms.
- Android/iOS now launch using new "ti.shell.js" instead of "app.js"
* Used to load JS extension/bootstrap scripts that are shared on all platforms.
* Loads "app.js" once finished loading everything.
- Now logs app version and Titanium version on startup via "ti.shell.js" instead of from native code.
* New Message: "<AppName> <AppVer> (Powered by Titanium <TiVer>.<TiBuildHash>)"
- [TIMOB-26015] Added bootstrap script support.
* All files named "bootstrap.js" or end with name "*.bootstrap.js" will be automatically required-in before "app.js" gets loaded.
* This is the CommonJS equivalent of native Android module's onAppCreate() or iOS module's load() method.
* Bootstrap script can support optional "exports.execute(finishedCallback)" method intended to display UI to end-user. Will not load "app.js" until given callback is invoked.
- [TIMOB-26124] Fixed bug where "Ti.buildHash" always returned null on Android as of Titanium 6.0.0.
* Was a typo in "scons-build.js" and "scons-cleanbuild.js" scripts.
- [TIMOB-26128] Modified Android build system to support including asset directory names have a leading underscore '_'.
* Solved by using "appt" command line argument "--ignore-assets" and omitting "<dir>_*" from the filter.
- [TIMOB-26118] Added ES6 String methods to iOS 8.
* Needed by new bootstrap script feature.
* Implemented via new JS extensions feature. (See: ./templates/core/Resources/_ti/extensions/String.js)
- [TIMOB-25899] Modified JS "Error" object to expose non-enumerable properties via JSON.stringify().
* Was always returning "{}" on Android since none of the V8 "Error" object properties are enumerable.
* Also modified to reveal "stack" property on iOS.
* Solved by adding toJSON() method to Error object prototype. (See: ./templates/core/Resources/_ti/extensions/Error.js)
There was a problem hiding this comment.
Some general comments:
- The location in the repo of these common JS files still feels odd to me. I'm not a fan of shoving it under a templates subfolder. If this is truly cross-platform public API, then it should probably be at a top-level folder as a sibling to iphone/android (i.e.
titanium_mobile/common) - Also, the name of
ti.shell.jsfeels off to me. - How do we handle errors in the bootstrapping? Looks like we don't really have any error-handling here, so I guess we basically blow up?
- Is there a noticeable performance hit? I know that Node basically writes the js code in "core" into a C array to avoid file I/O to improve perf there (and that's what we do in Android for some situations, I believe?)
- Do we anticipate platform-specific "bootstrap" code, or API extensions? Would they just get placed under platform specific sub-folders as in normal apps? We already have core JS code for Android, do we anticipate trying to incorporate that, or leaving it as-is?
I'm also concerned that we have multiple avenues of extending the app/API via common JS code - obviously there's CommonJS modules, but they wouldn't have the opportunity to be involved in the bootstrapping process right now. Is this expected to be limited to us SDK developers to be contributing these? It appears that a developer could potentially place files in a specific location in their app to play along.
| } | ||
|
|
||
| @Override | ||
| public boolean isDirectory() | ||
| { | ||
| if (statsFetched) { | ||
| return this.exists && this.typeDir; | ||
| if (this.statsFetched == false) { |
There was a problem hiding this comment.
Any particular reason there's a companions to false rather than typical !this.statsFetched?
| fetchBootstrapScriptsFrom(resourceDirectory); | ||
|
|
||
| // Sort the bootstrap scripts so that they'll be loaded in a consistent order between platforms. | ||
| bootstrapScripts.sort(); |
There was a problem hiding this comment.
Maybe some docs/help on what exactly the order will be? Obviously different OSes may sort differently naturally, so this is a potential point of confusion.
There was a problem hiding this comment.
I'm curious about that too. I'm not quite sure if calling JavaScript Array.sort() without any arguments guarantees the order across platforms actually. I see that Mozilla describes it's sorted according to Unicode code point and Microsoft sorts it in ASCII character order, and ECMAScript spec says it orders according to ToString() comparison (Do they describe same thing? I'm not sure). Maybe we might want to place compareFunction explicitly to avoid confusion.
There was a problem hiding this comment.
I was thinking that we want to give scripts a chance to define order of the bootstrap script by exposing priority like exports.priority but for now I'm almost fine with current implementation. I think simply sorting files according to file names would work well for developers. For instance in case developer wants to bootstrap network scripts before UI scripts, just rename bootstrap files like
001.something_to_boot_Network.js
002.something_to_boot_UI_theme.js
003.google_play.js
|
|
||
| // Fetches all "*.bootstrap.js" files under given directory and adds them to the "bootstrapScripts" array. | ||
| // The names stored in the array can be loaded via require() method. | ||
| function fetchBootstrapScriptsFrom(file) { |
There was a problem hiding this comment.
Why not move these function declarations out to top-level?
|
@sgtcoolguy, thanks for your feedback.
I agree. Although after doing a packing/install, a "common" folder already exists and we've put a "simplejson" tool there (I don't agree with this; should be under a "bin" or "tools" folder). How about I put the shared JS files under "titanium_mobile/core"?
The name seemed appropriate since Titanium is a JavaScript wrapper for the app and the OS' native APIs, similar to how a Unix shell provides a script interface to the OS' native APIs. The "ti.shell" wraps the "apps.js", controls how the app launches, and provides our own custom JS API extensions. I kind of like the name. Do you have another name in mind?
We don't. This is intentional and makes it easier to debug the bootstrap script. The
Fair question. I'll do some more measurements/testing and get back to you on this. As a quick summary for now, I have a test app that has about 160 JavaScript files in the root Resources directory. The amount of files in the directory listing is definitely a factor. The overhead is about 100 ms in the iOS Simulator and 140-1000 ms on various Android device. The 1 second overhead came from my slowest Android device that takes 10 seconds to start an app without my changes (I'm not happy about this). This can be optimized.
Platform Specific:
Shared:
I don't anticipate the "core/Resources" folder to be used for platform specific things except for core JS APIs that may have to do some if-android or if-ios types of things. It wouldn't be any worse than a CommonJS module. Ideally, we should keep platform specific code to a minimum. I did see our Android specific JavaScript code/extension and was going to leave that be for the moment.
The bootstrap concept is my JavaScript equivalent of our native Android module The "extensions" folder I created is a separate thing. This is only for us and is meant for us to add APIs to our core libraries/prototypes for all platforms. It's an opportunity for us to be more productive and improve parity. If a new feature can be 100% coded in JavaScript, then we should do it instead of implementing it natively on each platform we support. For example, if you want to extend "console" with new logging/performance APIs, then you can do it here and it'll just-work on all platforms. |
|
Regarding startup performance, I've determined that the @garymathews said he may look into moving the "simplejson" tool that's kept under the Titanium "common" directory. If it's only for Android module builds, then it belongs under the Android directory. That way I can move the core JS code I've created under the root "common" directory without risk of collision. Or alternatively I can name the directory "commonjs" to be more specific on what it is. |
|
I tried to integrate ti.shell.js on Windows, I made it working and the sample code ui.bootstrap.js works good locally. Looks good 👍 |
|
Thanks for your feedback @infosia. Would you mind doing a quick performance test for me on Windows please? Add about 100 files to your project and then use the following "ti.shell.js" code to log how long it takes. Note that if your build script copies the core scripts to the "build" directory before copying over the project's scripts, then you don't have to modify the "ti.shell.js" under "titanium_mobile". You can override it by adding a "ti.shell.js" to your Titanium app project instead since it would overwrite it. I consider this a feature and I use it to quick testing of the core scripts I've made (particularly the extensions). 'use strict';
Ti.API.info(Ti.App.name + ' ' + Ti.App.version + ' (Powered by Titanium ' + Ti.version + '.' + Ti.buildHash + ')');
var startTime = new Date();
require('./_ti/extension.loader').load();
Ti.API.info("@@@ Extension Loader Duration: " + (new Date() - startTime));
startTime = new Date();
require('./_ti/bootstrap.loader').loadAsync(function () {
Ti.API.info("@@@ Bootstrap Loader Duration: " + (new Date() - startTime));
require('./app');
});Ideally, I'd like to leave my bootstrap loader as-is and hunt down bootstrap files dynamically on startup... provided that the performance is fine on all platforms. Alternative solution is to modify our build tools to hunt down the bootstrap files when we do a build and store it to file for quick lookup on app startup instead, but I'm hoping to avoid that for the sake of simplicity. |
| @@ -0,0 +1,80 @@ | |||
| /** | |||
There was a problem hiding this comment.
We don't need this anymore due to tidev/node-titanium-sdk#32
Unless this is for example purposes?
|
Is there any special reason to use leading underscore |
|
@infosia, I'm using an On Android, only the "res" directory has character/name limitations since they're turned into Java constants. The Android "assets" directory (where we copy the app project's files) doesn't have any restrictions other than the filter list that is intended to exclude folders such as ".git", ".hg", ".svn", etc. I wasn't aware of any special treatment of leading underscores in WinRT. I'm guessing we're not adding these files under its equivalent "assets" directory? If we were to ever add images under the |
|
FYI Tried 100 bootstrap files on WindowsStore app (Intel Core i7-6700 CPU @ 3.40GHz, 16.0GB memory) and it took 14.5 seconds to load. |
|
How about using fully qualified name like Android packages? Such as |
|
@infosia, I'm sorry. I didn't mean for you to test 100 bootstrap files. I meant for you to add 100 other file types such as txt files. This is to test how fast (or slow) the recursive directory traversal is. For example, my app project contains about 200 files. About 25% of them are image files and most of the rest are JS files (not bootstrap related). I only have 2 bootstrap files in my project. (On Android, the Sorry for not being specific. |
|
Interestingly |
|
If we let the developer place the bootstrap-files in an own directory (like we plan for the SDK itself as well), wouldn't that cut down the number of files to search ? Right now, the performance would be a huge blocker. |
|
Ah I think I get it, Titanium CLI should be able to generate file lists of Resources directory beforehand at compile time (in JSON format?). That way we don't have to call |
…e in Resources directory. - Also improves performance of exists(), isFile(), and isDirectory() checks under Resources.
|
I've just updated the PR to improve Android's "Resources" file/directory access performance. Recursive directory traversal under the "Resources" directory is now about 7x faster than before. And the Here are the performance differences for startup times with my project having 200 files...
It's a pretty significant improvement and all Android devices (except Galaxy Nexus) now outperform iOS. The Galaxy Nexus is our slowest device in the office and takes about 10 seconds to launch an app normally (without this PR's changes), so, this is still a "win" on that device as well. Also, the emulator got the biggest performance gain because JS files are not encrypted and are instead dumped to the APK's "assets" directory which is what I just super optimized. @infosia, I'm going to see about adding optional support for a JSON of JavaScript files that the "_build.js" can create and bundle with the app. And if the JSON file is not there, then fallback to my existing @hansemannn, we need to be able to find bootstrap files belonging to modules and each module is put under their own subdirectory. Because of this, a recursive lookup is required. My master plan is to add a |
|
@jquick-axway I did quick trial to store file list of |
| @@ -3987,6 +3999,7 @@ AndroidBuilder.prototype.packageApp = function packageApp(next) { | |||
| '-S', this.buildResDir, | |||
| '-I', this.androidCompileSDK.androidJar, | |||
| '-F', this.ap_File, | |||
| '--ignore-assets', '!.svn:!.git:!.ds_store:!*.scc:.*:!CVS:!thumbs.db:!picasa.ini:!*~', | |||
There was a problem hiding this comment.
For a more complete list of things we ignore, check out https://github.com/appcelerator/titanium/blob/master/lib/config.js#L40-L41. It's probably overkill, but you did add picasa.ini for some reason.
Not sure if it helps, but you can get the regexes defined by those config settings by calling this.config.get('cli.ignoreDirs') and this.config.get('cli.ignoreFiles'). You'd need to convert it from a regex to that string with the negates which would be fun.
|
Updated PR:
|
|
Updated PR:
|
|
@jquick-axway @cb1kenobi @sgtcoolguy I want to use this functionality to load ACA on startup (TIMOB-25955) What's the status of this? I'd rather we not make use of Promises just yet until we adopt Promise support with our Titanium APIs which we can do at a later date. There are too many complications to address by supporting Promises that are beyond the scope of this PR. What's left to get this merged? |
Generated by 🚫 dangerJS |
|
@jquick-axway @garymathews I'm good with the current state of the PR. The only change I could see would be to remove the unnecessary String extensions for ES6. (We'll be transpiling by default, polyfilling, and in 8.0.0 we're dropping iOS 8 support) Josh, is it Ok to merge as-is? |
Tests:
Generated by 🚫 dangerJS |
|
@sgtcoolguy, I'll get rid of the "String.js" extension. Especially since (after all of my changes) I'm only using its |
|
Test comments in the appropriate tickets.
|
Generated by 🚫 dangerJS |
- Titanium now supports polyfill. That is expected to be used instead.
|
PR updated:
|
|
Jenkins build fails. Can someone please take a look at it. Thanks !! |
|
Tests:
Generated by 🚫 dangerJS |
Generated by 🚫 dangerJS |
JIRA:
Summary:
ti.main.jsinstead ofapp.js.app.jsonce finished loading everything.ti.main.jsinstead of from native code.<AppName> <AppVer> (Powered by Titanium <TiVer>.<TiBuildHash>)bootstrap.jsor end with name*.bootstrap.jsunder the "Resources" directory will be automatically required-in beforeapp.jsgets loaded. This is a recursive search.onAppCreate()or iOS module'sload()method.exports.execute(finishedCallback)method intended to display UI to end-user. Will not loadapp.jsuntil given callback is invoked.Ti.buildHashalways returnednullon Android as of Titanium 6.0.0.scons-build.jsandscons-cleanbuild.jsscripts.aaptAndroid command line tool.apptcommand line argument--ignore-assetsand omitting<dir>_*from filter list.Errorobject to expose non-enumerable properties viaJSON.stringify()."{}"on Android since none of the V8Errorobject properties are enumerable.stackproperty on iOS. (Not enumerable in JavaScriptCore.)toJSON()method toErrorobject prototype. (See:./common/Resources/ti.internal/extensions/Error.js)File.getDirectoryListing()performance is now about 20x faster. TheisDirectory(),isFile(), andexistence()calls have been improved too.AssetManager.list()calls.Ti.Filesystem.FilemethodsisFile()andisDirectory()when referencing an encrypted asset.File.read()to not return an empty string when referencing an encrypted asset.bootstrap.jsonfor device/production builds.Notes:
*.bootstrap.jshandling in our API docs. We should dog-food it first to see how much we like it before setting it in stone. Plus we may want it to usePromisesin the future (this feature is missing on iOS 9 at the moment).ti.playservicesmodule take advantage of the bootstrapping feature in 7.4.0. It's needed to check/install/update "Google Play Services" before loading theapp.js.Test:
Perform the below tests on the following:
Test procedure:
app.jsandui.bootstrap.jscode. Note that theui.bootstrap.jsneeds to be next to theapp.jsfile under theResourcesdirectory.app.jswill have loaded, displaying a JavaScriptErrorobject in JSON form. (This verifies that our core JS extensions have been loaded.)"{}". The JSON should display entries "message" and "stack".app.js
ui.bootstrap.js