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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-24830] (6_2_X) iOS: require cache improvements #9238

Merged
merged 5 commits into from
Jul 27, 2017
Merged

[TIMOB-24830] (6_2_X) iOS: require cache improvements #9238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
470a9d7
try to implement better caching for requires: cache currentURL+reques…
sgtcoolguy Jun 19, 2017
94476a6
Fix return type of many require methods to be KrollWrapper*; fix up p…
sgtcoolguy Jun 19, 2017
f79075a
When a require path is absolute, force path cache key to treat the cu…
sgtcoolguy Jun 19, 2017
97b3b55
Address review concerns
sgtcoolguy Jul 17, 2017
e2cef85
Update KrollBridge.m
hansemannn Jul 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions iphone/Classes/KrollBridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ extern NSString * TitaniumModuleRequireFormat;
KrollContext *context;
NSDictionary *preload;
NSMutableDictionary *modules;
NSMutableDictionary *pathCache;
TitaniumObject *titanium;
KrollObject* console;
KrollObject* console;
BOOL shutdown;
BOOL evaluationError;
BOOL evaluationError;
//NOTE: Do NOT treat registeredProxies like a mutableDictionary; mutable dictionaries copy keys,
//CFMutableDictionaryRefs only retain keys, which lets them work with proxies properly.
CFMutableDictionaryRef registeredProxies;
Expand All @@ -70,4 +71,3 @@ extern NSString * TitaniumModuleRequireFormat;
-(int)forceGarbageCollectNow;

@end

250 changes: 149 additions & 101 deletions iphone/Classes/KrollBridge.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ -(id)init
NSLog(@"[DEBUG] INIT: %@",self);
#endif
modules = [[NSMutableDictionary alloc] init];
pathCache = [[NSMutableDictionary alloc] init];
proxyLock = OS_SPINLOCK_INIT;
OSSpinLockLock(&krollBridgeRegistryLock);
CFSetAddValue(krollBridgeRegistry, self);
Expand Down Expand Up @@ -752,7 +753,7 @@ - (id)krollObjectForProxy:(id)proxy
return result;
}

-(id)loadCommonJSModule:(NSString*)code withSourceURL:(NSURL *)sourceURL
-(KrollWrapper *)loadCommonJSModule:(NSString*)code withSourceURL:(NSURL *)sourceURL
{
// This takes care of resolving paths like `../../foo.js`
sourceURL = [NSURL fileURLWithPath:[[sourceURL path] stringByStandardizingPath]];
Expand Down Expand Up @@ -819,7 +820,7 @@ - (TiModule *)loadTopLevelNativeModule:(TiModule *)module withPath:(NSString *)p

NSString* contents = [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] autorelease];
NSURL *url_ = [TiHost resourceBasedURL:path baseURL:NULL];
KrollWrapper *wrapper = (id) [self loadCommonJSModule:contents withSourceURL:url_];
KrollWrapper *wrapper = [self loadCommonJSModule:contents withSourceURL:url_];

// For right now, we need to mix any compiled JS on top of a compiled module, so that both components
// are accessible. We store the exports object and then put references to its properties on the toplevel
Expand All @@ -845,7 +846,7 @@ - (TiModule *)loadTopLevelNativeModule:(TiModule *)module withPath:(NSString *)p
return module;
}

- (TiModule *)loadCoreModule:(NSString *)path withContext:(KrollContext *)kroll
- (id)loadCoreModule:(NSString *)path withContext:(KrollContext *)kroll
{
// make sure path doesn't begin with ., .., or /
// Can't be a "core" module then
Expand Down Expand Up @@ -905,7 +906,7 @@ - (TiModule *)loadCoreModule:(NSString *)path withContext:(KrollContext *)kroll
// nope, return nil so we can try to fall back to resource in user's app
return nil;
}
NSString* contents = [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] autorelease];
NSString* contents = [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] autorelease];
// This is an asset inside the native module. Load it like a "normal" common js file
return [self loadJavascriptText:contents fromFile:filepath withContext:kroll];
}
Expand All @@ -926,19 +927,8 @@ - (NSString *)loadFile:(NSString *)path
return nil;
}

- (TiModule *)loadJavascriptObject:(NSString *)data fromFile:(NSString *)filename withContext:(KrollContext *)kroll
- (KrollWrapper *)loadJavascriptObject:(NSString *)data fromFile:(NSString *)filename withContext:(KrollContext *)kroll
{
// FIXME Move this up the stack to where we check for file existence?

// Now that we have the full path, we can check and see if the module was loaded,
// and return it if available.
if (modules != nil) {
TiModule *module = [modules objectForKey:filename];
if (module != nil) {
return module;
}
}

// We could cheat and just do "module.exports = %data%", but that wouldn't validate that the passed in content was JSON
// and may open a security hole.

Expand All @@ -959,19 +949,8 @@ - (TiModule *)loadJavascriptObject:(NSString *)data fromFile:(NSString *)filenam
return [self loadJavascriptText:data fromFile:filename withContext:kroll];
}

- (TiModule *)loadJavascriptText:(NSString *)data fromFile:(NSString *)filename withContext:(KrollContext *)kroll
- (KrollWrapper *)loadJavascriptText:(NSString *)data fromFile:(NSString *)filename withContext:(KrollContext *)kroll
{
// FIXME Move this up the stack to where we check for file existence?

// Now that we have the full path, we can check and see if the module was loaded,
// and return it if available.
if (modules != nil) {
TiModule *module = [modules objectForKey:filename];
if (module != nil) {
return module;
}
}

NSURL *url_ = [TiHost resourceBasedURL:filename baseURL:NULL];
#ifndef USE_JSCORE_FRAMEWORK
const char *urlCString = [[url_ absoluteString] UTF8String];
Expand All @@ -991,12 +970,12 @@ - (TiModule *)loadJavascriptText:(NSString *)data fromFile:(NSString *)filename

if (![wrapper respondsToSelector:@selector(replaceValue:forKey:notification:)]) {
@throw [NSException exceptionWithName:@"org.appcelerator.kroll"
reason:[NSString stringWithFormat:@"Module \"%@\" failed to leave a valid exports object", filename]
reason:[NSString stringWithFormat:@"Module \"%@\" failed to leave a valid exports object", filename]
userInfo:nil];
}

// register the module if it's pure JS
TiModule *module = (id)wrapper;
KrollWrapper *module = (id)wrapper;

// cache the module by filename
[modules setObject:module forKey:filename];
Expand All @@ -1009,37 +988,61 @@ - (TiModule *)loadJavascriptText:(NSString *)data fromFile:(NSString *)filename
return module;
}

- (TiModule *)loadAsFile:(NSString *)path withContext:(KrollContext *)kroll
- (KrollWrapper *)cachedLoadAsFile:(NSString *)path asJSON:(BOOL)json withContext:(KrollContext *)kroll
{
// 1. If X is a file, load X as JavaScript text. STOP
NSString *filename = path;
NSString *data = [self loadFile:filename];
// check cache first
if (modules != nil) {
KrollWrapper *module = [modules objectForKey:path];
if (module != nil) {
return module;
}
}

// Fall back to trying to load file
NSString *data = [self loadFile:path];
if (data != nil) {
// If the file extension is .json, load as JavascriptObject!
NSString *ext = [filename pathExtension];
if (ext != nil && [ext isEqual:@"json"]) {
return [self loadJavascriptObject:data fromFile:filename withContext:context];
if (json) {
return [self loadJavascriptObject:data fromFile:path withContext:context];
}
return [self loadJavascriptText:data fromFile:filename withContext:context];
return [self loadJavascriptText:data fromFile:path withContext:context];
}
return nil;
}

- (KrollWrapper *)loadAsFile:(NSString *)path withContext:(KrollContext *)kroll
{
NSString *filename = path;

// 1. If X is a file, load X as JavaScript text. STOP
// Note: I modified the algorithm here to handle .json files as JSON, everything else as JS
NSString *ext = [filename pathExtension];
BOOL json = (ext != nil && [ext isEqual:@"json"]);
KrollWrapper *module = [self cachedLoadAsFile:filename asJSON:json withContext:context];
if (module != nil) {
return module;
}

// 2. If X.js is a file, load X.js as JavaScript text. STOP
filename = [path stringByAppendingString:@".js"];
data = [self loadFile:filename];
if (data != nil) {
return [self loadJavascriptText:data fromFile:filename withContext:context];
module = [self cachedLoadAsFile:filename asJSON:NO withContext:context];
if (module != nil) {
return module;
}

// 3. If X.json is a file, parse X.json to a JavaScript Object. STOP
filename = [path stringByAppendingString:@".json"];
data = [self loadFile:filename];
if (data != nil) {
return [self loadJavascriptObject:data fromFile:filename withContext:context];
module = [self cachedLoadAsFile:filename asJSON:YES withContext:context];
if (module != nil) {
return module;
}

// failed to load anything!
return nil;
}

- (TiModule *)loadAsDirectory:(NSString *)path withContext:(KrollContext *)kroll
- (KrollWrapper *)loadAsDirectory:(NSString *)path withContext:(KrollContext *)kroll
{
// FIXME Use loadJavascriptObject: or cachedLoadAsFile: to get package.json and then get the main value out of it?
// 1. If X/package.json is a file,
NSString *filename = [path stringByAppendingPathComponent:@"package.json"];
NSString *data = [self loadFile:filename];
Expand All @@ -1062,25 +1065,35 @@ - (TiModule *)loadAsDirectory:(NSString *)path withContext:(KrollContext *)kroll

// 2. If X/index.js is a file, load X/index.js as JavaScript text. STOP
filename = [path stringByAppendingPathComponent:@"index.js"];
data = [self loadFile:filename];
if (data != nil) {
return [self loadJavascriptText:data fromFile:filename withContext:context];
KrollWrapper *module = [self cachedLoadAsFile:filename asJSON:NO withContext:context];
if (module != nil) {
return module;
}

// 3. If X/index.json is a file, parse X/index.json to a JavaScript object. STOP
filename = [path stringByAppendingPathComponent:@"index.json"];
data = [self loadFile:filename];
if (data != nil) {
return [self loadJavascriptObject:data fromFile:filename withContext:context];
module = [self cachedLoadAsFile:filename asJSON:YES withContext:context];
if (module != nil) {
return module;
}

return nil;
}

- (TiModule *)loadAsFileOrDirectory:(NSString *)path withContext:(KrollContext *)kroll
- (KrollWrapper *)loadAsFileOrDirectory:(NSString *)path withContext:(KrollContext *)kroll
{
TiModule *module = nil;
// FIXME Can we improve perf a little here by detecting if the target is a file or directory first?
// i.e.
// - if node_modules/whatever exists and is a dir, we can skip checking for node_modules/whatever.js at least
// - if it doesn't exist at all, we can skip checking:
// - node_modules/whatever
// - node_modules/whatever/index.js
// - node_modules/whatever/package.json
// - node_modules/whatever/index.json
// - node_modules/whatever/whatever.js

// a. LOAD_AS_FILE(Y + X)
module = [self loadAsFile:path withContext:context];
KrollWrapper *module = [self loadAsFile:path withContext:context];
if (module) {
return module;
}
Expand Down Expand Up @@ -1130,9 +1143,9 @@ - (NSArray *)nodeModulesPaths:(NSString *)path
return dirs;
}

- (TiModule *)loadNodeModules:(NSString *)path withDir:(NSString *)start withContext:(KrollContext *)kroll
- (KrollWrapper *)loadNodeModules:(NSString *)path withDir:(NSString *)start withContext:(KrollContext *)kroll
{
TiModule *module = nil;
KrollWrapper *module = nil;

// 1. let DIRS=NODE_MODULES_PATHS(START)
NSArray *dirs = [self nodeModulesPaths:start];
Expand All @@ -1152,65 +1165,100 @@ - (TiModule *)loadNodeModules:(NSString *)path withDir:(NSString *)start withCon
- (id)require:(KrollContext *)kroll path:(NSString *)path
{
NSURL *oldURL = [self currentURL];
NSString *workingPath = [oldURL relativePath];
NSMutableString *pathCacheKey;
@try {
// 1. If X is a core module,
TiModule *module = [self loadCoreModule:path withContext:kroll];
if (module) {
// a. return the core module
// b. STOP
return module;
}

// 2. If X begins with './' or '/' or '../'
if ([path hasPrefix:@"./"] || [path hasPrefix:@"../"]) {
// Need base path to work from for relative modules...
NSString *workingPath = [oldURL relativePath];
NSString *relativePath = (workingPath == nil) ? path : [workingPath stringByAppendingPathComponent:path];
module = [self loadAsFileOrDirectory:[relativePath stringByStandardizingPath] withContext:context];
if (module) {
return module;
// First let's check if we cached the resolved path for this require string
// and if we did, try and load a cached module for this path
if (pathCache != nil && modules != nil) {
// We generate a path resolution cache key, first part is the requested module id/path
pathCacheKey = [path stringByAppendingString:@"|"];
// If request is not-absolute and we're not at the top-level dir, then append current dir as second part of cache key
if (workingPath != nil && ![path hasPrefix:@"/"]) {
pathCacheKey = [pathCacheKey stringByAppendingString: workingPath];
}
NSString *resolvedPath = [pathCache objectForKey:pathCacheKey];
if (resolvedPath != nil) {
TiModule *module = [modules objectForKey:resolvedPath];
if (module != nil) {
return module;
}
}
// Treat '/' special as absolute, drop the leading '/'
}
else if ([path hasPrefix:@"/"]) {
module = [self loadAsFileOrDirectory:[[path substringFromIndex:1] stringByStandardizingPath] withContext:context];

id module; // may be TiModule* if it was a core module with no hybrid JS, or KrollWrapper* in all other cases
@try {
// 1. If X is a core module,
module = [self loadCoreModule:path withContext:kroll];
if (module) {
// a. return the core module
// b. STOP
return module;
}
} else {
// TODO Grab the first path segment and see if it's a node module or commonJS module
// We should be able to organize the modules in folder to determine if the user is attempting to
// load one of them!


// Look for CommonJS module
if (![path containsString:@"/"]) {
// For CommonJS we need to look for module.id/module.id.js first...
// TODO Only look for this _exact file_. DO NOT APPEND .js or .json to it!
module = [self loadAsFile:[[path stringByAppendingPathComponent:path] stringByAppendingPathExtension:@"js"] withContext:context];
// 2. If X begins with './' or '/' or '../'
if ([path hasPrefix:@"./"] || [path hasPrefix:@"../"]) {
// Need base path to work from for relative modules...
NSString *relativePath = (workingPath == nil) ? path : [workingPath stringByAppendingPathComponent:path];
module = [self loadAsFileOrDirectory:[relativePath stringByStandardizingPath] withContext:context];
if (module) {
return module;
}
// Then try module.id as directory
module = [self loadAsDirectory:path withContext:context];
// Treat '/' special as absolute, drop the leading '/'
}
else if ([path hasPrefix:@"/"]) {
module = [self loadAsFileOrDirectory:[[path substringFromIndex:1] stringByStandardizingPath] withContext:context];
if (module) {
return module;
}
}
} else {
// TODO Grab the first path segment and see if it's a node module or commonJS module
// We should be able to organize the modules in folder to determine if the user is attempting to
// load one of them!

// Look for CommonJS module
if (![path containsString:@"/"]) {
// For CommonJS we need to look for module.id/module.id.js first...
// Only look for this _exact file_. DO NOT APPEND .js or .json to it!
NSString *filename = [[path stringByAppendingPathComponent:path] stringByAppendingPathExtension:@"js"];
module = [self cachedLoadAsFile:filename asJSON:NO withContext:context];
if (module) {
return module;
}

// Then try module.id as directory
module = [self loadAsDirectory:path withContext:context];
if (module) {
return module;
}
}

// Need base path to work from for determining the node_modules search paths.
NSString *workingPath = [oldURL relativePath];
module = [self loadNodeModules:path withDir:workingPath withContext:context];
if (module) {
return module;
}
// Need base path to work from for determining the node_modules search paths.
module = [self loadNodeModules:path withDir:workingPath withContext:context];
if (module) {
return module;
}

// We'd like to warn users about legacy style require syntax so they can update, but the new syntax is not backwards compatible.
// So for now, let's just be quite about it. In future versions of the SDK (7.0?) we should warn (once 5.x is end of life so backwards compat is not necessary)
//NSLog(@"require called with un-prefixed module id: %@, should be a core or CommonJS module. Falling back to old Ti behavior and assuming it's an absolute path: /%@", path, path);
module = [self loadAsFileOrDirectory:[path stringByStandardizingPath] withContext:context];
if (module) {
return module;
// We'd like to warn users about legacy style require syntax so they can update, but the new syntax is not backwards compatible.
// So for now, let's just be quite about it. In future versions of the SDK (7.0?) we should warn (once 5.x is end of life so backwards compat is not necessary)
//NSLog(@"require called with un-prefixed module id: %@, should be a core or CommonJS module. Falling back to old Ti behavior and assuming it's an absolute path: /%@", path, path);
module = [self loadAsFileOrDirectory:[path stringByStandardizingPath] withContext:context];
if (module) {
return module;
}
}
}
@finally {
// Cache the resolved path for this request if we got a module
if (module != nil && pathCache != nil && pathCacheKey != nil) {
// I cannot find a nicer way of grabbing the filepath out of the "id" or "uri" properties for the module!
NSArray *keys = [modules allKeysForObject:module];
if (keys != nil) {
NSString *filename = keys[0];
if (filename) { // native modules may have no value
[pathCache setObject:filename forKey:pathCacheKey];
}
}
}
}
}
Expand Down