Skip to content

Make Function.prototype.toString() source text retention opt-in (#2560)#2562

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:increased-memory
Jul 2, 2026
Merged

Make Function.prototype.toString() source text retention opt-in (#2560)#2562
lahma merged 1 commit into
sebastienros:mainfrom
lahma:increased-memory

Conversation

@lahma

@lahma lahma commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #2560.

Problem

Since v4.9.0 (#2394), Function.prototype.toString() returns the real source text of user functions. To support this, every parsed function node pins the entire source string (Node.UserData = ctx.Input), released only lazily on the first toString() call. Applications that cache many large prepared scripts and never call toString() therefore retain the full source for the lifetime of the cache — the reporter observed ~250 MB of duplicated source strings (the same ~500 KB string retained once per engine).

Fix

Add an opt-in RetainFunctionSourceText option, default false:

  • IParsingOptions.RetainFunctionSourceText on ScriptParsingOptions / ModuleParsingOptions — controls per-parse retention (prepared and non-prepared).
  • Options.RetainFunctionSourceText (+ fluent options.RetainFunctionSourceText()) — engine-level toggle for the default parser, so bare engine.Execute(string) can opt in without passing parsing options.

When retention is disabled (the default), source text is not kept and toString() returns the pre-4.9 function name() { [native code] } placeholder. When enabled, real source is returned exactly as before.

// lean by default now — no source retained:
new Engine().Evaluate("function f(){} f.toString()"); // "function f() { [native code] }"

// opt back in:
new Engine(o => o.RetainFunctionSourceText()).Evaluate("function f(){} f.toString()"); // real source
Engine.PrepareScript(code, options: new ScriptPreparationOptions {
    ParsingOptions = new ScriptParsingOptions { RetainFunctionSourceText = true } });

Behavior change: this reverts the v4.9 default (real-source toString()) to opt-in, trading it for lower memory by default. The Test262 harness and the existing toString source tests opt in so conformance is unchanged.

Verification

  • Jint.Tests: 3147 passed / 0 failed (excluding the pre-existing, environment-sensitive EngineLimitTests.ShouldAllowReasonableCallStackDepth, which crashes on unmodified main here too).
  • Test262: 99260 passed / 0 failed — the built-ins/Function/prototype/toString/* conformance tests stay green via the harness opt-in.
  • Retained memory (GarbageCollectionTests.PreparedScriptsDoNotRetainSourceTextByDefault): holding 25 distinct ~800 KB cached prepared scripts retains 20.1 MB with retention on vs 1.8 MB off — ~18.3 MB / 91% freed.

The secondary env-pooling suspect (#2413) is left for a separate, benchmarked PR; it is a bounded, self-healing-under-load retention, unlike the quantified source-string regression fixed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

…stienros#2560)

Since v4.9.0 (sebastienros#2394) every parsed function node pins the entire source
string (Node.UserData = ctx.Input) so Function.prototype.toString() can
return the original source, released only lazily on the first toString()
call. Applications that cache many large prepared scripts and never call
toString() therefore retain hundreds of MB of duplicated source strings
(issue sebastienros#2560: ~250 MB, the same ~500 KB string per engine).

Add an opt-in RetainFunctionSourceText parsing option (default false) plus
an engine-level Options.RetainFunctionSourceText toggle. When disabled (the
default) the source text is not retained and toString() returns the
"function name() { [native code] }" placeholder.

- RetainFunctionSourceText on IParsingOptions + ScriptParsingOptions/ModuleParsingOptions
- ApplyTo wires OnNode conditionally; prepared path passes null/ctx.Input in AstAnalyzer
- Options.RetainFunctionSourceText + fluent extension; engine default parser honors it
- Test262 harness and the sebastienros#2394 source-asserting tests opt in to keep conformance green
- Retained-memory regression test: 25 cached ~800 KB scripts retain 20.1 MB vs 1.8 MB

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9
@lahma
lahma enabled auto-merge (squash) July 2, 2026 13:59
@lahma
lahma merged commit 682d677 into sebastienros:main Jul 2, 2026
4 checks passed
@lahma
lahma deleted the increased-memory branch July 2, 2026 15:41
@scgm0

scgm0 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Can't we just keep the source text once, have the node store its own slice position, and only create the string slice from the source text based on the position when calling toString()?

@lahma

lahma commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

You can always propose a PR, but sounds that we are still pinning the source string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increased memory use compared to v4.4.0 when using many large prepared scripts

2 participants