Skip to content

Commit 49adb7e

Browse files
author
Mihail Slavchev
committed
refactor Module.cpp to use instance method and cache instead of static ones
1 parent c4e024f commit 49adb7e

4 files changed

Lines changed: 108 additions & 122 deletions

File tree

‎src/jni/Module.cpp‎

Lines changed: 78 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,23 @@ using namespace v8;
2424
using namespace std;
2525
using namespace tns;
2626

27+
Module::Module()
28+
: m_isolate(nullptr), m_requireFunction(nullptr), m_requireFactoryFunction(nullptr)
29+
{
30+
}
31+
2732
void Module::Init(Isolate *isolate)
2833
{
2934
JEnv env;
3035

31-
MODULE_CLASS = env.FindClass("com/tns/Module");
32-
assert(MODULE_CLASS != nullptr);
36+
if (MODULE_CLASS == nullptr)
37+
{
38+
MODULE_CLASS = env.FindClass("com/tns/Module");
39+
assert(MODULE_CLASS != nullptr);
3340

34-
RESOLVE_PATH_METHOD_ID = env.GetStaticMethodID(MODULE_CLASS, "resolvePath", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;");
35-
assert(RESOLVE_PATH_METHOD_ID != nullptr);
41+
RESOLVE_PATH_METHOD_ID = env.GetStaticMethodID(MODULE_CLASS, "resolvePath", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;");
42+
assert(RESOLVE_PATH_METHOD_ID != nullptr);
43+
}
3644

3745
string requireFactoryScript =
3846
"(function () { "
@@ -53,6 +61,8 @@ void Module::Init(Isolate *isolate)
5361
auto source = ConvertToV8String(requireFactoryScript);
5462
auto context = isolate->GetCurrentContext();
5563

64+
auto global = context->Global();
65+
5666
Local<Script> script;
5767
auto maybeScript = Script::Compile(context, source).ToLocal(&script);
5868

@@ -65,15 +75,13 @@ void Module::Init(Isolate *isolate)
6575

6676
auto requireFactoryFunction = result.As<Function>();
6777

68-
auto cache = GetCache(isolate);
78+
m_requireFactoryFunction = new Persistent<Function>(isolate, requireFactoryFunction);
6979

70-
cache->RequireFactoryFunction = new Persistent<Function>(isolate, requireFactoryFunction);
71-
72-
auto requireFuncTemplate = FunctionTemplate::New(isolate, RequireCallback);
80+
auto requireFuncTemplate = FunctionTemplate::New(isolate, RequireCallback, External::New(isolate, this));
7381
auto requireFunc = requireFuncTemplate->GetFunction();
74-
cache->RequireFunction = new Persistent<Function>(isolate, requireFunc);
82+
global->Set(ConvertToV8String("__nativeRequire"), requireFunc);
83+
m_requireFunction = new Persistent<Function>(isolate, requireFunc);
7584

76-
auto global = isolate->GetCurrentContext()->Global();
7785
auto globalRequire = GetRequireFunction(isolate, Constants::APP_ROOT_FOLDER_PATH);
7886
global->Set(ConvertToV8String("require"), globalRequire);
7987
}
@@ -82,25 +90,24 @@ Local<Function> Module::GetRequireFunction(Isolate *isolate, const string& dirNa
8290
{
8391
Local<Function> requireFunc;
8492

85-
auto cache = GetCache(isolate);
86-
87-
auto itFound = cache->RequireCache.find(dirName);
93+
auto itFound = m_requireCache.find(dirName);
8894

89-
if (itFound != cache->RequireCache.end())
95+
if (itFound != m_requireCache.end())
9096
{
9197
requireFunc = Local<Function>::New(isolate, *itFound->second);
9298
}
9399
else
94100
{
95-
auto requireFuncFactory = Local<Function>::New(isolate, *cache->RequireFactoryFunction);
101+
auto requireFuncFactory = Local<Function>::New(isolate, *m_requireFactoryFunction);
96102

97103
auto context = isolate->GetCurrentContext();
98104

99-
auto requireInternalFunc = Local<Function>::New(isolate, *cache->RequireFunction);
105+
auto requireInternalFunc = Local<Function>::New(isolate, *m_requireFunction);
100106

101107
Local<Value> args[2]
102108
{
103-
requireInternalFunc, ConvertToV8String(dirName) };
109+
requireInternalFunc, ConvertToV8String(dirName)
110+
};
104111
Local<Value> result;
105112
auto thiz = Object::New(isolate);
106113
auto success = requireFuncFactory->Call(context, thiz, 2, args).ToLocal(&result);
@@ -111,7 +118,7 @@ Local<Function> Module::GetRequireFunction(Isolate *isolate, const string& dirNa
111118

112119
auto poFunc = new Persistent<Function>(isolate, requireFunc);
113120

114-
cache->RequireCache.insert(make_pair(dirName, poFunc));
121+
m_requireCache.insert(make_pair(dirName, poFunc));
115122
}
116123

117124
return requireFunc;
@@ -121,50 +128,8 @@ void Module::RequireCallback(const v8::FunctionCallbackInfo<v8::Value>& args)
121128
{
122129
try
123130
{
124-
auto isolate = args.GetIsolate();
125-
126-
if (args.Length() != 2)
127-
{
128-
throw NativeScriptException(string("require should be called with two parameters"));
129-
}
130-
if (!args[0]->IsString())
131-
{
132-
throw NativeScriptException(string("require's first parameter should be string"));
133-
}
134-
if (!args[1]->IsString())
135-
{
136-
throw NativeScriptException(string("require's second parameter should be string"));
137-
}
138-
139-
string moduleName = ConvertToString(args[0].As<String>());
140-
string callingModuleDirName = ConvertToString(args[1].As<String>());
141-
142-
JEnv env;
143-
JniLocalRef jsModulename(env.NewStringUTF(moduleName.c_str()));
144-
JniLocalRef jsCallingModuleDirName(env.NewStringUTF(callingModuleDirName.c_str()));
145-
JniLocalRef jsModulePath(env.CallStaticObjectMethod(MODULE_CLASS, RESOLVE_PATH_METHOD_ID, (jstring) jsModulename, (jstring) jsCallingModuleDirName));
146-
147-
// cache the required modules by full path, not name only, since there might be some collisions with relative paths and names
148-
string modulePath = ArgConverter::jstringToString((jstring) jsModulePath);
149-
150-
auto isData = false;
151-
152-
auto moduleObj = LoadImpl(isolate, modulePath, isData);
153-
154-
if (isData)
155-
{
156-
assert(!moduleObj.IsEmpty());
157-
158-
args.GetReturnValue().Set(moduleObj);
159-
}
160-
else
161-
{
162-
auto exportsObj = moduleObj->Get(ConvertToV8String("exports"));
163-
164-
assert(!exportsObj.IsEmpty());
165-
166-
args.GetReturnValue().Set(exportsObj);
167-
}
131+
auto thiz = static_cast<Module*>(args.Data().As<External>()->Value());
132+
thiz->RequireCallbackImpl(args);
168133
}
169134
catch (NativeScriptException& e)
170135
{
@@ -182,6 +147,54 @@ void Module::RequireCallback(const v8::FunctionCallbackInfo<v8::Value>& args)
182147
}
183148
}
184149

150+
void Module::RequireCallbackImpl(const v8::FunctionCallbackInfo<v8::Value>& args)
151+
{
152+
auto isolate = args.GetIsolate();
153+
154+
if (args.Length() != 2)
155+
{
156+
throw NativeScriptException(string("require should be called with two parameters"));
157+
}
158+
if (!args[0]->IsString())
159+
{
160+
throw NativeScriptException(string("require's first parameter should be string"));
161+
}
162+
if (!args[1]->IsString())
163+
{
164+
throw NativeScriptException(string("require's second parameter should be string"));
165+
}
166+
167+
string moduleName = ConvertToString(args[0].As<String>());
168+
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+
178+
auto isData = false;
179+
180+
auto moduleObj = LoadImpl(isolate, modulePath, isData);
181+
182+
if (isData)
183+
{
184+
assert(!moduleObj.IsEmpty());
185+
186+
args.GetReturnValue().Set(moduleObj);
187+
}
188+
else
189+
{
190+
auto exportsObj = moduleObj->Get(ConvertToV8String("exports"));
191+
192+
assert(!exportsObj.IsEmpty());
193+
194+
args.GetReturnValue().Set(exportsObj);
195+
}
196+
}
197+
185198
void Module::RequireNativeCallback(const v8::FunctionCallbackInfo<v8::Value>& args)
186199
{
187200
auto ext = args.Data().As<External>();
@@ -208,11 +221,9 @@ Local<Object> Module::LoadImpl(Isolate *isolate, const string& path, bool& isDat
208221
{
209222
Local<Object> result;
210223

211-
auto cache = GetCache(isolate);
224+
auto it = m_loadedModules.find(path);
212225

213-
auto it = cache->LoadedModules.find(path);
214-
215-
if (it == cache->LoadedModules.end())
226+
if (it == m_loadedModules.end())
216227
{
217228
if (Util::EndsWith(path, ".js") || Util::EndsWith(path, ".so"))
218229
{
@@ -251,7 +262,7 @@ Local<Object> Module::LoadModule(Isolate *isolate, const string& modulePath)
251262
moduleObj->Set(ConvertToV8String("filename"), fullRequiredModulePath);
252263

253264
auto poModuleObj = new Persistent<Object>(isolate, moduleObj);
254-
TempModule tempModule(isolate, modulePath, poModuleObj);
265+
TempModule tempModule(this, modulePath, poModuleObj);
255266

256267
TryCatch tc;
257268

@@ -458,25 +469,8 @@ void Module::SaveScriptCache(const ScriptCompiler::Source& source, const std::st
458469
File::WriteBinary(cachePath, source.GetCachedData()->data, length);
459470
}
460471

461-
Module::Cache* Module::GetCache(Isolate *isolate)
462-
{
463-
Cache *cache;
464-
auto itFound = s_cache.find(isolate);
465-
if (itFound == s_cache.end())
466-
{
467-
cache = new Cache;
468-
s_cache.insert(make_pair(isolate, cache));
469-
}
470-
else
471-
{
472-
cache = itFound->second;
473-
}
474-
return cache;
475-
}
476-
477472
jclass Module::MODULE_CLASS = nullptr;
478473
jmethodID Module::RESOLVE_PATH_METHOD_ID = nullptr;
479474

480475
const char* Module::MODULE_PROLOGUE = "(function(module, exports, require, __filename, __dirname){ ";
481476
const char* Module::MODULE_EPILOGUE = "\n})";
482-
map<Isolate*, Module::Cache*> Module::s_cache;

‎src/jni/Module.h‎

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,70 +19,60 @@ namespace tns
1919
class Module
2020
{
2121
public:
22-
static void Init(v8::Isolate *isolate);
22+
Module();
2323

24-
static void Load(const std::string& path);
24+
void Init(v8::Isolate *isolate);
2525

26-
static void RequireCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
26+
void Load(const std::string& path);
2727

2828
private:
29-
struct Cache;
29+
static void RequireCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
3030

31-
Module() = default;
31+
static void RequireNativeCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
3232

33-
static Cache* GetCache(v8::Isolate *isolate);
33+
void RequireCallbackImpl(const v8::FunctionCallbackInfo<v8::Value>& args);
3434

35-
static v8::Local<v8::String> WrapModuleContent(const std::string& path);
35+
v8::Local<v8::String> WrapModuleContent(const std::string& path);
3636

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

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

41-
static v8::Local<v8::Object> LoadData(v8::Isolate *isolate, const std::string& path);
41+
v8::Local<v8::Object> LoadData(v8::Isolate *isolate, const std::string& path);
4242

43-
static v8::Local<v8::Script> LoadScript(v8::Isolate *isolate, const std::string& modulePath, const v8::Local<v8::String>& fullRequiredModulePath);
43+
v8::Local<v8::Script> LoadScript(v8::Isolate *isolate, const std::string& modulePath, const v8::Local<v8::String>& fullRequiredModulePath);
4444

45-
static void RequireNativeCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
45+
v8::Local<v8::Function> GetRequireFunction(v8::Isolate *isolate, const std::string& dirName);
4646

47-
static v8::Local<v8::Function> GetRequireFunction(v8::Isolate *isolate, const std::string& dirName);
47+
v8::ScriptCompiler::CachedData* TryLoadScriptCache(const std::string& path);
4848

49-
static v8::ScriptCompiler::CachedData* TryLoadScriptCache(const std::string& path);
50-
static void SaveScriptCache(const v8::ScriptCompiler::Source& source, const std::string& path);
49+
void SaveScriptCache(const v8::ScriptCompiler::Source& source, const std::string& path);
5150

5251
static jclass MODULE_CLASS;
5352
static jmethodID RESOLVE_PATH_METHOD_ID;
54-
5553
static const char* MODULE_PROLOGUE;
5654
static const char* MODULE_EPILOGUE;
57-
static std::map<v8::Isolate*, Cache*> s_cache;
5855

59-
struct Cache
60-
{
61-
v8::Persistent<v8::Function> *RequireFunction;
62-
63-
v8::Persistent<v8::Function> *RequireFactoryFunction;
64-
65-
std::map<std::string, v8::Persistent<v8::Function>*> RequireCache;
66-
67-
std::map<std::string, v8::Persistent<v8::Object>*> LoadedModules;
68-
};
56+
v8::Isolate *m_isolate;
57+
v8::Persistent<v8::Function> *m_requireFunction;
58+
v8::Persistent<v8::Function> *m_requireFactoryFunction;
59+
std::map<std::string, v8::Persistent<v8::Function>*> m_requireCache;
60+
std::map<std::string, v8::Persistent<v8::Object>*> m_loadedModules;
6961

7062
class TempModule
7163
{
7264
public:
73-
TempModule(v8::Isolate *isolate, const std::string& modulePath, v8::Persistent<v8::Object> *poModuleObj)
74-
:m_isolate(isolate), m_dispose(true), m_modulePath(modulePath), m_poModuleObj(poModuleObj)
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)
7567
{
76-
auto cache = GetCache(isolate);
77-
cache->LoadedModules.insert(make_pair(m_modulePath, m_poModuleObj));
68+
m_module->m_loadedModules.insert(make_pair(m_modulePath, m_poModuleObj));
7869
}
7970

8071
~TempModule()
8172
{
8273
if (m_dispose)
8374
{
84-
auto cache = GetCache(m_isolate);
85-
cache->LoadedModules.erase(m_modulePath);
75+
m_module->m_loadedModules.erase(m_modulePath);
8676
}
8777
}
8878

@@ -93,7 +83,7 @@ namespace tns
9383

9484
private:
9585
bool m_dispose;
96-
v8::Isolate *m_isolate;
86+
Module *m_module;
9787
std::string m_modulePath;
9888
v8::Persistent<v8::Object> *m_poModuleObj;
9989
};

‎src/jni/Runtime.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ void Runtime::RunModule(JNIEnv *_env, jobject obj, jstring scriptFile)
149149
JEnv env(_env);
150150

151151
string filePath = ArgConverter::jstringToString(scriptFile);
152-
Module::Load(filePath);
152+
m_module.Load(filePath);
153153
}
154154

155155
jobject Runtime::RunScript(JNIEnv *_env, jobject obj, jstring scriptFile)
@@ -450,7 +450,6 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, jstring packageName,
450450
globalTemplate->Set(ConvertToV8String("__enableVerboseLogging"), FunctionTemplate::New(isolate, CallbackHandlers::EnableVerboseLoggingMethodCallback));
451451
globalTemplate->Set(ConvertToV8String("__disableVerboseLogging"), FunctionTemplate::New(isolate, CallbackHandlers::DisableVerboseLoggingMethodCallback));
452452
globalTemplate->Set(ConvertToV8String("__exit"), FunctionTemplate::New(isolate, CallbackHandlers::ExitMethodCallback));
453-
globalTemplate->Set(ConvertToV8String("__nativeRequire"), FunctionTemplate::New(isolate, Module::RequireCallback));
454453
globalTemplate->Set(ConvertToV8String("__runtimeVersion"), ConvertToV8String(NATIVE_SCRIPT_RUNTIME_VERSION), readOnlyFlags);
455454

456455
m_weakRef.Init(isolate, globalTemplate, m_objectManager);
@@ -466,9 +465,9 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, jstring packageName,
466465

467466
m_objectManager->Init(isolate);
468467

469-
auto global = context->Global();
468+
m_module.Init(isolate);
470469

471-
Module::Init(isolate);
470+
auto global = context->Global();
472471

473472
global->ForceSet(ConvertToV8String("global"), global, readOnlyFlags);
474473
global->ForceSet(ConvertToV8String("__global"), global, readOnlyFlags);

0 commit comments

Comments
 (0)