Skip to content

Commit 52d010b

Browse files
author
Mihail Slavchev
committed
Cache modules by unique combination of module path and basedir. This works well for global modules and helps with common requires. Essentially, the currently used map now contains more than one key (path) for a given module
1 parent 49adb7e commit 52d010b

2 files changed

Lines changed: 91 additions & 32 deletions

File tree

‎src/jni/Module.cpp‎

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -166,18 +166,9 @@ void Module::RequireCallbackImpl(const v8::FunctionCallbackInfo<v8::Value>& args
166166

167167
string moduleName = ConvertToString(args[0].As<String>());
168168
string callingModuleDirName = ConvertToString(args[1].As<String>());
169-
170-
JEnv env;
171-
JniLocalRef jsModulename(env.NewStringUTF(moduleName.c_str()));
172-
JniLocalRef jsCallingModuleDirName(env.NewStringUTF(callingModuleDirName.c_str()));
173-
JniLocalRef jsModulePath(env.CallStaticObjectMethod(MODULE_CLASS, RESOLVE_PATH_METHOD_ID, (jstring) jsModulename, (jstring) jsCallingModuleDirName));
174-
175-
// cache the required modules by full path, not name only, since there might be some collisions with relative paths and names
176-
string modulePath = ArgConverter::jstringToString((jstring) jsModulePath);
177-
178169
auto isData = false;
179170

180-
auto moduleObj = LoadImpl(isolate, modulePath, isData);
171+
auto moduleObj = LoadImpl(isolate, moduleName, callingModuleDirName, isData);
181172

182173
if (isData)
183174
{
@@ -217,40 +208,64 @@ void Module::Load(const string& path)
217208
}
218209
}
219210

220-
Local<Object> Module::LoadImpl(Isolate *isolate, const string& path, bool& isData)
211+
Local<Object> Module::LoadImpl(Isolate *isolate, const string& moduleName, const string& baseDir, bool& isData)
221212
{
213+
auto pathKind = GetModulePathKind(moduleName);
214+
auto cachePathKey = (pathKind == ModulePathKind::Global) ? moduleName : (baseDir + "*" + moduleName);
215+
222216
Local<Object> result;
223217

224-
auto it = m_loadedModules.find(path);
218+
DEBUG_WRITE(">>LoadImpl cachePathKey=%s", cachePathKey.c_str());
219+
220+
auto it = m_loadedModules.find(cachePathKey);
225221

226222
if (it == m_loadedModules.end())
227223
{
228-
if (Util::EndsWith(path, ".js") || Util::EndsWith(path, ".so"))
229-
{
230-
isData = false;
231-
result = LoadModule(isolate, path);
232-
}
233-
else if (Util::EndsWith(path, ".json"))
224+
JEnv env;
225+
JniLocalRef jsModulename(env.NewStringUTF(moduleName.c_str()));
226+
JniLocalRef jsBaseDir(env.NewStringUTF(baseDir.c_str()));
227+
JniLocalRef jsModulePath(env.CallStaticObjectMethod(MODULE_CLASS, RESOLVE_PATH_METHOD_ID, (jstring) jsModulename, (jstring) jsBaseDir));
228+
229+
auto path = ArgConverter::jstringToString((jstring) jsModulePath);
230+
231+
auto it2 = m_loadedModules.find(path);
232+
233+
if (it2 == m_loadedModules.end())
234234
{
235-
isData = true;
236-
result = LoadData(isolate, path);
235+
if (Util::EndsWith(path, ".js") || Util::EndsWith(path, ".so"))
236+
{
237+
isData = false;
238+
result = LoadModule(isolate, path, cachePathKey);
239+
}
240+
else if (Util::EndsWith(path, ".json"))
241+
{
242+
isData = true;
243+
result = LoadData(isolate, path);
244+
}
245+
else
246+
{
247+
string errMsg = "Unsupported file extension: " + path;
248+
throw NativeScriptException(errMsg);
249+
}
237250
}
238251
else
239252
{
240-
string errMsg = "Unsupported file extension: " + path;
241-
throw NativeScriptException(errMsg);
253+
auto& cacheEntry = it2->second;
254+
isData = cacheEntry.isData;
255+
result = Local<Object>::New(isolate, *cacheEntry.obj);
242256
}
243257
}
244258
else
245259
{
246-
isData = Util::EndsWith(path, ".json");
247-
result = Local<Object>::New(isolate, *it->second);
260+
auto& cacheEntry = it->second;
261+
isData = cacheEntry.isData;
262+
result = Local<Object>::New(isolate, *cacheEntry.obj);
248263
}
249264

250265
return result;
251266
}
252267

253-
Local<Object> Module::LoadModule(Isolate *isolate, const string& modulePath)
268+
Local<Object> Module::LoadModule(Isolate *isolate, const string& modulePath, const string& moduleCacheKey)
254269
{
255270
Local<Object> result;
256271

@@ -262,7 +277,7 @@ Local<Object> Module::LoadModule(Isolate *isolate, const string& modulePath)
262277
moduleObj->Set(ConvertToV8String("filename"), fullRequiredModulePath);
263278

264279
auto poModuleObj = new Persistent<Object>(isolate, moduleObj);
265-
TempModule tempModule(this, modulePath, poModuleObj);
280+
TempModule tempModule(this, modulePath, moduleCacheKey, poModuleObj);
266281

267282
TryCatch tc;
268283

@@ -469,6 +484,18 @@ void Module::SaveScriptCache(const ScriptCompiler::Source& source, const std::st
469484
File::WriteBinary(cachePath, source.GetCachedData()->data, length);
470485
}
471486

487+
Module::ModulePathKind Module::GetModulePathKind(const std::string& path)
488+
{
489+
ModulePathKind kind;
490+
switch (path[0])
491+
{
492+
case '.': kind = ModulePathKind::Relative; break;
493+
case '/': kind = ModulePathKind::Absolute; break;
494+
default: kind = ModulePathKind::Global; break;
495+
}
496+
return kind;
497+
}
498+
472499
jclass Module::MODULE_CLASS = nullptr;
473500
jmethodID Module::RESOLVE_PATH_METHOD_ID = nullptr;
474501

‎src/jni/Module.h‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ namespace tns
2626
void Load(const std::string& path);
2727

2828
private:
29+
enum class ModulePathKind;
30+
31+
struct ModuleCacheEntry;
32+
2933
static void RequireCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
3034

3135
static void RequireNativeCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -34,9 +38,9 @@ namespace tns
3438

3539
v8::Local<v8::String> WrapModuleContent(const std::string& path);
3640

37-
v8::Local<v8::Object> LoadImpl(v8::Isolate *isolate, const std::string& path, bool& isData);
41+
v8::Local<v8::Object> LoadImpl(v8::Isolate *isolate, const std::string& moduleName, const std::string& baseDir, bool& isData);
3842

39-
v8::Local<v8::Object> LoadModule(v8::Isolate *isolate, const std::string& path);
43+
v8::Local<v8::Object> LoadModule(v8::Isolate *isolate, const std::string& path, const std::string& moduleCacheKey);
4044

4145
v8::Local<v8::Object> LoadData(v8::Isolate *isolate, const std::string& path);
4246

@@ -48,6 +52,8 @@ namespace tns
4852

4953
void SaveScriptCache(const v8::ScriptCompiler::Source& source, const std::string& path);
5054

55+
ModulePathKind GetModulePathKind(const std::string& path);
56+
5157
static jclass MODULE_CLASS;
5258
static jmethodID RESOLVE_PATH_METHOD_ID;
5359
static const char* MODULE_PROLOGUE;
@@ -57,22 +63,24 @@ namespace tns
5763
v8::Persistent<v8::Function> *m_requireFunction;
5864
v8::Persistent<v8::Function> *m_requireFactoryFunction;
5965
std::map<std::string, v8::Persistent<v8::Function>*> m_requireCache;
60-
std::map<std::string, v8::Persistent<v8::Object>*> m_loadedModules;
66+
std::map<std::string, ModuleCacheEntry> m_loadedModules;
6167

6268
class TempModule
6369
{
6470
public:
65-
TempModule(Module* module, const std::string& modulePath, v8::Persistent<v8::Object> *poModuleObj)
66-
:m_module(module), m_dispose(true), m_modulePath(modulePath), m_poModuleObj(poModuleObj)
71+
TempModule(Module* module, const std::string& modulePath, const std::string& cacheKey, v8::Persistent<v8::Object> *poModuleObj)
72+
:m_module(module), m_dispose(true), m_modulePath(modulePath), m_cacheKey(cacheKey), m_poModuleObj(poModuleObj)
6773
{
68-
m_module->m_loadedModules.insert(make_pair(m_modulePath, m_poModuleObj));
74+
m_module->m_loadedModules.insert(make_pair(m_modulePath, ModuleCacheEntry(m_poModuleObj)));
75+
m_module->m_loadedModules.insert(make_pair(m_cacheKey, ModuleCacheEntry(m_poModuleObj)));
6976
}
7077

7178
~TempModule()
7279
{
7380
if (m_dispose)
7481
{
7582
m_module->m_loadedModules.erase(m_modulePath);
83+
m_module->m_loadedModules.erase(m_cacheKey);
7684
}
7785
}
7886

@@ -85,8 +93,32 @@ namespace tns
8593
bool m_dispose;
8694
Module *m_module;
8795
std::string m_modulePath;
96+
std::string m_cacheKey;
8897
v8::Persistent<v8::Object> *m_poModuleObj;
8998
};
99+
100+
struct ModuleCacheEntry
101+
{
102+
ModuleCacheEntry(v8::Persistent<v8::Object> *_obj)
103+
: obj(_obj), isData(false)
104+
{
105+
}
106+
107+
ModuleCacheEntry(v8::Persistent<v8::Object> *_obj, bool _isData)
108+
: obj(_obj), isData(_isData)
109+
{
110+
}
111+
112+
bool isData;
113+
v8::Persistent<v8::Object> *obj;
114+
};
115+
116+
enum class ModulePathKind
117+
{
118+
Global,
119+
Relative,
120+
Absolute
121+
};
90122
};
91123
}
92124

0 commit comments

Comments
 (0)