Skip to content

Commit

Permalink
Bug 1845584 - Remove JS::CompilationStorage parameter from JS::Prepar…
Browse files Browse the repository at this point in the history
…eForInstantiate. r=bthrall

Differential Revision: https://phabricator.services.mozilla.com/D184663
  • Loading branch information
arai-a committed Aug 10, 2023
1 parent ec821c6 commit 18f30d5
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 63 deletions.
4 changes: 2 additions & 2 deletions js/public/experimental/CompileScript.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ extern JS_PUBLIC_API already_AddRefed<JS::Stencil> CompileModuleScriptToStencil(
JS::SourceText<char16_t>& srcBuf, JS::CompilationStorage& compileStorage);

extern JS_PUBLIC_API bool PrepareForInstantiate(
JS::FrontendContext* fc, JS::CompilationStorage& compileStorage,
JS::Stencil& stencil, JS::InstantiationStorage& storage);
JS::FrontendContext* fc, JS::Stencil& stencil,
JS::InstantiationStorage& storage);

} // namespace JS

Expand Down
4 changes: 2 additions & 2 deletions js/public/experimental/JSStencil.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ struct InstantiationStorage {
InstantiationStorage* storage);

friend JS_PUBLIC_API bool PrepareForInstantiate(
JS::FrontendContext* fc, JS::CompilationStorage& compileStorage,
JS::Stencil& stencil, JS::InstantiationStorage& storage);
JS::FrontendContext* fc, JS::Stencil& stencil,
JS::InstantiationStorage& storage);

friend struct js::ParseTask;

Expand Down
2 changes: 1 addition & 1 deletion js/src/builtin/TestingFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6963,7 +6963,7 @@ static bool CompileToStencil(JSContext* cx, uint32_t argc, Value* vp) {

JS::InstantiationStorage storage;
if (prepareForInstantiate) {
if (!JS::PrepareForInstantiate(&fc, compileStorage, *stencil, storage)) {
if (!JS::PrepareForInstantiate(&fc, *stencil, storage)) {
return false;
}
}
Expand Down
3 changes: 1 addition & 2 deletions js/src/frontend/CompilationStencil.h
Original file line number Diff line number Diff line change
Expand Up @@ -1252,8 +1252,7 @@ struct CompilationStencil {
FrontendContext* fc, CompilationAtomCache& atomCache,
const CompilationStencil& stencil, CompilationGCOutput& gcOutput);
[[nodiscard]] static bool prepareForInstantiate(
FrontendContext* fc, CompilationAtomCache& atomCache,
const CompilationStencil& stencil,
FrontendContext* fc, const CompilationStencil& stencil,
PreallocatedCompilationGCOutput& gcOutput);

[[nodiscard]] static bool instantiateStencils(
Expand Down
26 changes: 3 additions & 23 deletions js/src/frontend/CompileScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,28 +189,8 @@ already_AddRefed<JS::Stencil> JS::CompileModuleScriptToStencil(
compileStorage);
}

#ifdef DEBUG
// We don't need to worry about GC if the CompilationInput has no GC pointers
static bool isGCSafe(js::frontend::CompilationInput& input) {
bool isGlobalOrModule =
input.target == CompilationInput::CompilationTarget::Global ||
input.target == CompilationInput::CompilationTarget::Module;
bool scopeHasNoGC =
input.enclosingScope.isStencil() || input.enclosingScope.isNull();
bool scriptHasNoGC =
input.lazyOuterScript().isStencil() || input.lazyOuterScript().isNull();
bool cacheHasNoGC = input.atomCache.empty();

return isGlobalOrModule && scopeHasNoGC && scriptHasNoGC && cacheHasNoGC;
}
#endif // DEBUG

bool JS::PrepareForInstantiate(JS::FrontendContext* fc,
JS::CompilationStorage& compileStorage,
JS::Stencil& stencil,
bool JS::PrepareForInstantiate(JS::FrontendContext* fc, JS::Stencil& stencil,
JS::InstantiationStorage& storage) {
MOZ_ASSERT(compileStorage.hasInput());
MOZ_ASSERT(isGCSafe(compileStorage.getInput()));
if (!storage.gcOutput_) {
storage.gcOutput_ =
fc->getAllocator()
Expand All @@ -219,6 +199,6 @@ bool JS::PrepareForInstantiate(JS::FrontendContext* fc,
return false;
}
}
return CompilationStencil::prepareForInstantiate(
fc, compileStorage.getInput().atomCache, stencil, *storage.gcOutput_);
return CompilationStencil::prepareForInstantiate(fc, stencil,
*storage.gcOutput_);
}
11 changes: 3 additions & 8 deletions js/src/frontend/Stencil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2910,15 +2910,10 @@ bool CompilationStencil::prepareForInstantiate(

/* static */
bool CompilationStencil::prepareForInstantiate(
FrontendContext* fc, CompilationAtomCache& atomCache,
const CompilationStencil& stencil,
FrontendContext* fc, const CompilationStencil& stencil,
PreallocatedCompilationGCOutput& gcOutput) {
if (!gcOutput.allocate(fc, stencil.scriptData.size(),
stencil.scopeData.size())) {
return false;
}

return atomCache.allocate(fc, stencil.parserAtomData.size());
return gcOutput.allocate(fc, stencil.scriptData.size(),
stencil.scopeData.size());
}

bool CompilationStencil::serializeStencils(JSContext* cx,
Expand Down
3 changes: 1 addition & 2 deletions js/src/jsapi-tests/testCompileScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ bool testPrepareForInstantiate() {
CHECK(compileStorage.getInput().atomCache.empty());

JS::InstantiationStorage storage;
CHECK(JS::PrepareForInstantiate(fc, compileStorage, *stencil, storage));
CHECK(compileStorage.getInput().atomCache.size() == 1); // 'field'
CHECK(JS::PrepareForInstantiate(fc, *stencil, storage));
CHECK(storage.isValid());
// TODO storage.gcOutput_ is private, so there isn't a good way to check the
// scriptData and scopeData capacities
Expand Down
30 changes: 7 additions & 23 deletions js/src/vm/HelperThreads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,7 @@ void CompileToStencilTask<Unit>::parse(FrontendContext* fc) {
}

if (options.allocateInstantiationStorage) {
if (!JS::PrepareForInstantiate(fc, compileStorage_, *stencil_,
instantiationStorage_)) {
if (!JS::PrepareForInstantiate(fc, *stencil_, instantiationStorage_)) {
stencil_ = nullptr;
}
}
Expand All @@ -620,8 +619,7 @@ void CompileModuleToStencilTask<Unit>::parse(FrontendContext* fc) {
}

if (options.allocateInstantiationStorage) {
if (!JS::PrepareForInstantiate(fc, compileStorage_, *stencil_,
instantiationStorage_)) {
if (!JS::PrepareForInstantiate(fc, *stencil_, instantiationStorage_)) {
stencil_ = nullptr;
}
}
Expand All @@ -637,29 +635,16 @@ DecodeStencilTask::DecodeStencilTask(JSContext* cx,
}

void DecodeStencilTask::parse(FrontendContext* fc) {
if (!compileStorage_.allocateInput(fc, options)) {
return;
}
if (!compileStorage_.getInput().initForGlobal(fc)) {
return;
}

stencil_ = fc->getAllocator()->new_<frontend::CompilationStencil>(
compileStorage_.getInput().source);
if (!stencil_) {
return;
}
JS::DecodeOptions decodeOptions(options);

bool succeeded = false;
(void)stencil_->deserializeStencils(fc, options, range, &succeeded);
if (!succeeded) {
stencil_ = nullptr;
JS::TranscodeResult tr =
JS::DecodeStencil(fc, decodeOptions, range, getter_AddRefs(stencil_));
if (tr != JS::TranscodeResult::Ok) {
return;
}

if (options.allocateInstantiationStorage) {
if (!JS::PrepareForInstantiate(fc, compileStorage_, *stencil_,
instantiationStorage_)) {
if (!JS::PrepareForInstantiate(fc, *stencil_, instantiationStorage_)) {
stencil_ = nullptr;
}
}
Expand Down Expand Up @@ -1839,7 +1824,6 @@ GlobalHelperThreadState::finishStencilTask(JSContext* cx,
return nullptr;
}

MOZ_ASSERT(parseTask->compileStorage_.hasInput());
MOZ_ASSERT(parseTask->stencil_.get());

if (storage) {
Expand Down

0 comments on commit 18f30d5

Please sign in to comment.