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

[TIMOB-17892] iOS: Update Titanium to use Apple's JavaScriptCore that ships with iOS. #6450

Closed
wants to merge 2 commits into from
Closed

[TIMOB-17892] iOS: Update Titanium to use Apple's JavaScriptCore that ships with iOS. #6450

wants to merge 2 commits into from

Conversation

matt-langston
Copy link
Contributor

DO NOT MERGE BEFORE PEER REVIEW

This PR updates the iOS Titanium SDK to use the JavaScriptCore that Apple ships with iOS. As such, I have removed libTiCore.a and all references to tijscore from the Titanium SDK (i.e. project files, SCons, cli build hooks, etc.) since Apple's JavaScriptCore.framework provides this functionality.

This PR resolve issues pertaining to TIMOB-11093, particularly the recent crashers TIMOB-18137, TIMOB-18135 and TIMOB-18118. It can also resolve TIMOB-18131 quite easily (i.e. the "missing stack trace info" problem - see screenshots below) but I did not want to pursue further development on this effort without peer review and critical feedback on this "Apple JavaScriptCore" approach.

A few notes:

  1. The goal of this PR is to demonstrate that the iOS Titanium SDK can use the JavaScriptCore.framework that ships with iOS. I have tested on both iOS Simulator and Device (iPhone 5s) for iOS 7.0, 7.1, 8.0 and 8.1.
  2. This PR resolves the three P1 crashers TIMOB-18137, TIMOB-18135 and TIMOB-18118.
  3. This PR demonstrates that Apple gives us rich stack trace information, and therefore can resolve TIMOB-18131 with minor effort.
  4. I was able to easily provide efficient implementations of TiValueIsArray and TiValueIsDate in the Titanium SDK itself using only Apple's public JavaScriptCore API (previously these were provided by modifications to tijscore in TiValueRef.h and TiValueRef.cpp that used private API).
  5. I am able to successfully use Apple's JavaScript debugger to debug Titanium applications (i.e. I can set breakpoints, inspect the call stack, variables, scripts, etc. - everything our Studio debugger provides to the best of my knowledge (however, I am no Studio debugger expert). Apple's debugger also lets me manipulate the running JavaScript of an iOS Titanium Application in a "Live View" type fashion.
  6. This PR should provide access to Apple's new profiler as well (a very rich, sophisticated profiler I might add), but I have yet to explore this effort because I need peer review and critical feedback on this "Apple JavaScriptCore" approach.

A few screenshot that demonstrate the power (and simplicity) of using Apple's WebInspector to debug an iOS Titanium application:

screenshot 2014-12-07 10 41 41

screenshot 2014-12-07 10 40 39

screenshot 2014-12-07 10 29 19

screenshot 2014-12-07 10 27 38

@farwayer
Copy link

farwayer commented Dec 7, 2014

Fucking crazy excellent work! :)

@jhaynie
Copy link
Contributor

jhaynie commented Dec 7, 2014

@matt-langston This is really good work and the changes are a lot less invasive than I thought they would be. There are obvious great opportunities to moving to this library once we can. I had discussed with both @ingo and @negupta (and @vishalduggal) yesterday my reluctance to make this change in 3.5. However, looking at the PR, I would be willing to reconsider and support this with the following conditions below. @ingo and @negupta will need to endorse this as well (and they may have their own conditions/concerns I've missed). I would also like to have @vishalduggal sign off on this change and be comfortable with the plan.

Here are my comments and requests for this PR specifically. These are conditions for this merging in 3.5 release. If we cannot satisfy all of these conditions, let's wait to merge this until after 3.5 is out in a subsequent 3.5.1 release (I would still request the same conditions regardless of the release).

I would like this not to be the default library when building your app. I would like the following to happen before you can use this library:

  • You must set any of the following.... a property in tiapp.xml to turn this on at the project level (following our standard convention for stuff like this) or command line flag for titanium build command. By setting either of these settings, the CLI would set a predefined compiler macro when invoking XCodebuild. It would also do the conditional work above (some of which was removed for libTiCore) in the CLI.
  • When the setting is off (default), it would use libTiCore.a
  • When the setting is on and you are not building for profiling or debug, it would use the Apple JavaScriptCore library. If you are building for profile or debugging, it would use libTiCore.a
  • In either case, we would add a log line indicating which library we are using at INFO log level to let developer (and support if an issue arises) know which one we're compiling with.
  • All changes in C/C++ and Objective-C would use the defined preprocessor compiler macro above to do the conditional changes necessary to make this PR work.
  • Changes to our Performance SDK (and other native libraries as required) to make sure it works with both versions of stack and backtrace on exceptions.
  • We may need to build with libTiCore.a if you're using third-party native modules (not our own) built prior to 3.5. I'm not sure if a previously built module will work anyway. However, given that we're now requiring all modules to be recompiled for 64-bit, maybe this is not as big of a deal. However, I'm worried that if you build for modules against libTiCore based headers and then try and then attempt to use with new Apple JavaScriptCore you will get linker errors (I'm quite positive but haven't of course tested this theory). One approach would simply be to force a build against libTiCore if you use modules until we can get module developers to build against the new library. Maybe we could provide a scheme where they can package both versions as part of packaging a module and we can include the right one when we compile the application. Should be very easy to test this out as part of validating this PR.

I think we can ask developers to live without profiler and debugger initially when building with the library (and we'll fall back to our libJSCore to do that if you run either of these as mentioned above) until we can officially support them with the new library and tools.

The plan would be that we would ask developers (as part of the release) to try switching to this library (with instructions how to enable) and report any issues (and success!). Assuming all goes well for awhile, we would then deprecate libTiCore in 3.6 (that would simply mean making Apple JavaScriptCore default and libTiCore optional) and then removing it entirely in 3.7. (Release numbers subject to change dependent on when it gets introduced and when feedback is meaningful enough to warrant moving to it as default).

@jhaynie
Copy link
Contributor

jhaynie commented Dec 8, 2014

OK, I spoke with Neeraj and I think we have a plan. This PR will merge after 3.5 since we are code complete today and we are trying to get this release out ASAP because of 64-bit requirement from Apple. This PR will merge after that and be available in subsequent release.

@viezel
Copy link

viezel commented Jan 17, 2015

any status on this?

@Core-13
Copy link

Core-13 commented Feb 27, 2015

Bump

@nuno
Copy link

nuno commented Feb 27, 2015

+1

4 similar comments
@mattapperson
Copy link

+1

@drmas
Copy link

drmas commented Mar 1, 2015

+1

@tujoworker
Copy link

+1

@WillDent
Copy link

WillDent commented Mar 2, 2015

+1

@kopiro
Copy link
Contributor

kopiro commented Mar 4, 2015

+2

@dbankier
Copy link
Contributor

See this: https://twitter.com/sebmarkbage/status/575059848083058688
Might be further motivation.

@jhaynie
Copy link
Contributor

jhaynie commented Mar 10, 2015

we have a ton of motivation, just right now trying to contend with a few technical issues (like debugger, etc) before we can merge this. this is VERY Important to us we just need to complete some of stuff quickly before we can

@ingo ingo changed the title [TIMOB-11093] iOS: Update Titanium to use Apple's JavaScriptCore that ships with iOS. TIMOB-17892] iOS: Update Titanium to use Apple's JavaScriptCore that ships with iOS. Mar 10, 2015
@ingo ingo changed the title TIMOB-17892] iOS: Update Titanium to use Apple's JavaScriptCore that ships with iOS. [TIMOB-17892] iOS: Update Titanium to use Apple's JavaScriptCore that ships with iOS. May 7, 2015
@ingo
Copy link
Contributor

ingo commented May 7, 2015

This PR was a prototype and was not merged, but a PR based on this work was merged in as part of https://jira.appcelerator.org/browse/TIMOB-17892. Please test this as part of the 4.1.0 master branch.

@ingo ingo closed this May 7, 2015
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.

None yet