Skip to content

Commit 70a6df2

Browse files
fix(script): drop AST after script execution (boa-dev#4876)
## Context This PR extends the idea introduced in boa-dev#4855. PR boa-dev#4855 fixed a memory retention issue for **modules**, where the parsed AST and source text were kept in memory even after module initialization. This PR applies the same idea to **scripts**, ensuring that the parsed AST is released once script execution has completed. # Summary This PR prevents scripts from keeping their parsed AST after compilation. Right now `Script::Inner` stores the full AST in the `source` field: ```rust struct Inner { realm: Realm, source: boa_ast::Script, source_text: SourceText, codeblock: GcRefCell<Option<Gc<CodeBlock>>>, } ``` The AST is only needed while generating bytecode in `Script::codeblock()`. After compilation it is not used anymore, but it still stays in `Script::Inner`. Function objects created from the script keep a reference to the script (`OrdinaryFunction::script_or_module`). Because of this, the script can stay alive for a long time, which also keeps the whole AST in memory. # Steps to Reproduce In a long-running program (for example a REPL or server), repeatedly run scripts that define functions: ```rust let script = Script::parse(Source::from_bytes(src), None, &mut context)?; script.evaluate(&mut context)?; ``` If those functions are stored on the global object, they keep the script alive. Since the script holds the AST, memory usage keeps growing as more scripts run. # Impact This mostly affects long-running programs that embed Boa. Every script that defines functions can keep its AST in memory forever. AST structures are usually much bigger than the source code, so memory usage can grow over time. Modules already drop their AST after linking, but scripts did not. # Fix `Script::Inner::source` is changed to: ```rust source: RefCell<Option<boa_ast::Script>> ``` The AST is used during compilation and then removed once the `CodeBlock` is created: ```rust *self.inner.source.borrow_mut() = None; ``` # Result After this change, the AST is dropped once compilation finishes. Scripts now behave consistently with modules after the change introduced in boa-dev#4855. **The AST is only required during parsing and compilation. Once the script finishes execution and the result is consumed, it can be safely released to reduce memory usage in long-running runtimes.** --------- Signed-off-by: ashnaaseth2325-oss <ashnaaseth2325@gmail.com> Co-authored-by: José Julián Espina <julian.espina@canonical.com> Co-authored-by: José Julián Espina <jedel0124@gmail.com>
1 parent bef1acb commit 70a6df2

1 file changed

Lines changed: 58 additions & 49 deletions

File tree

core/engine/src/script.rs

Lines changed: 58 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88
//! [spec]: https://tc39.es/ecma262/#sec-scripts
99
//! [script]: https://tc39.es/ecma262/#sec-script-records
1010
11-
use std::path::{Path, PathBuf};
11+
use std::{
12+
cell::RefCell,
13+
path::{Path, PathBuf},
14+
};
1215

1316
use rustc_hash::FxHashMap;
1417

@@ -37,19 +40,22 @@ impl std::fmt::Debug for Script {
3740
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
3841
f.debug_struct("Script")
3942
.field("realm", &self.inner.realm.addr())
40-
.field("code", &self.inner.source)
43+
.field("phase", &self.inner.phase.borrow())
4144
.field("loaded_modules", &self.inner.loaded_modules)
4245
.finish()
4346
}
4447
}
45-
48+
#[derive(Debug, Finalize)]
49+
enum ScriptPhase {
50+
Ast(boa_ast::Script),
51+
Codeblock(Gc<CodeBlock>),
52+
}
4653
#[derive(Trace, Finalize)]
4754
struct Inner {
4855
realm: Realm,
4956
#[unsafe_ignore_trace]
50-
source: boa_ast::Script,
57+
phase: RefCell<ScriptPhase>,
5158
source_text: SourceText,
52-
codeblock: GcRefCell<Option<Gc<CodeBlock>>>,
5359
loaded_modules: GcRefCell<FxHashMap<JsString, Module>>,
5460
host_defined: HostDefined,
5561
path: Option<PathBuf>,
@@ -102,9 +108,8 @@ impl Script {
102108
Ok(Self {
103109
inner: Gc::new(Inner {
104110
realm: realm.unwrap_or_else(|| context.realm().clone()),
105-
source: code,
111+
phase: RefCell::new(ScriptPhase::Ast(code)),
106112
source_text,
107-
codeblock: GcRefCell::default(),
108113
loaded_modules: GcRefCell::default(),
109114
host_defined: HostDefined::default(),
110115
path,
@@ -116,48 +121,50 @@ impl Script {
116121
///
117122
/// This is a no-op if this has been called previously.
118123
pub fn codeblock(&self, context: &mut Context) -> JsResult<Gc<CodeBlock>> {
119-
let mut codeblock = self.inner.codeblock.borrow_mut();
120-
121-
if let Some(codeblock) = &*codeblock {
122-
return Ok(codeblock.clone());
123-
}
124-
125-
let mut annex_b_function_names = Vec::new();
126-
127-
global_declaration_instantiation_context(
128-
&mut annex_b_function_names,
129-
&self.inner.source,
130-
self.inner.realm.scope(),
131-
context,
132-
)?;
133-
134-
let spanned_source_text = SpannedSourceText::new_source_only(self.get_source());
135-
let mut compiler = ByteCompiler::new(
136-
js_string!("<main>"),
137-
self.inner.source.strict(),
138-
false,
139-
self.inner.realm.scope().clone(),
140-
self.inner.realm.scope().clone(),
141-
false,
142-
false,
143-
context.interner_mut(),
144-
false,
145-
spanned_source_text,
146-
self.path().map(Path::to_owned).into(),
147-
);
148-
149-
#[cfg(feature = "annex-b")]
150-
{
151-
compiler.annex_b_function_names = annex_b_function_names;
152-
}
153-
154-
// TODO: move to `Script::evaluate` to make this operation infallible.
155-
compiler.global_declaration_instantiation(&self.inner.source);
156-
compiler.compile_statement_list(self.inner.source.statements(), true, false);
157-
158-
let cb = Gc::new(compiler.finish());
159-
160-
*codeblock = Some(cb.clone());
124+
let cb = {
125+
let phase = self.inner.phase.borrow();
126+
let source = match &*phase {
127+
ScriptPhase::Codeblock(codeblock) => return Ok(codeblock.clone()),
128+
ScriptPhase::Ast(source) => source,
129+
};
130+
131+
let mut annex_b_function_names = Vec::new();
132+
133+
global_declaration_instantiation_context(
134+
&mut annex_b_function_names,
135+
source,
136+
self.inner.realm.scope(),
137+
context,
138+
)?;
139+
140+
let spanned_source_text = SpannedSourceText::new_source_only(self.get_source());
141+
142+
let mut compiler = ByteCompiler::new(
143+
js_string!("<main>"),
144+
source.strict(),
145+
false,
146+
self.inner.realm.scope().clone(),
147+
self.inner.realm.scope().clone(),
148+
false,
149+
false,
150+
context.interner_mut(),
151+
false,
152+
spanned_source_text,
153+
self.path().map(Path::to_owned).into(),
154+
);
155+
156+
#[cfg(feature = "annex-b")]
157+
{
158+
compiler.annex_b_function_names = annex_b_function_names;
159+
}
160+
161+
compiler.global_declaration_instantiation(source);
162+
compiler.compile_statement_list(source.statements(), true, false);
163+
164+
Gc::new(compiler.finish())
165+
};
166+
167+
*self.inner.phase.borrow_mut() = ScriptPhase::Codeblock(cb.clone());
161168

162169
Ok(cb)
163170
}
@@ -173,6 +180,7 @@ impl Script {
173180
let record = context.run();
174181

175182
context.vm.pop_frame();
183+
176184
record.consume()
177185
}
178186

@@ -205,6 +213,7 @@ impl Script {
205213
let record = context.run_async_with_budget(budget).await;
206214

207215
context.vm.pop_frame();
216+
208217
record.consume()
209218
}
210219

0 commit comments

Comments
 (0)