Avoid creating JSFunction.object for functions stored in JSScript

RESOLVED WONTFIX

Status

()

Core
JavaScript Engine
--
enhancement
RESOLVED WONTFIX
11 years ago
10 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

(Depends on: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
[This is a pin-off of bug 398219 comment 11.]

Currently for each compiled function in a script the compiler creates JSFunction together with associated JSObject wrapper stored in JSFunction.object. With the bug 399544 fixed that object serves no essential purposes and it would be nice to optimize it away.

The current code may give impression that the wrapper object is used to store the parent scope chain. That allows later to skip cloning the wrapper when executing a script in some specific cases as an optimization. But if the wrapper object would not be created in the first place, the optimizations would be unnecessary.

Unfortunately not creating JSFunction.object breaks API compatibility as currently an embedding that calls JS_GetFunctionObject expects to get a non-null answer. On the other hand the price of such compatibility is relatively high. For example, after the browser startup with a trunk build from 2007-12-01 there are about 2600 such unused object that persists many GC cycles (see comment 1 for details). It wastes 2600*33 (size of JSObject + flags) or 83K of memory. So as a work-around a compiler or runtime option can be added that would enable skipping creating JSFunction.object when an embedding can deal with consequences.
(Assignee)

Comment 1

11 years ago
Created attachment 291013 [details] [diff] [review]
patch to measure the number of unused JSFunction.object

The patch writes a log about unused JSFunction.object to /tmp/ifun.log. The patch is build on top of the patch from bug 398219 comment 15 as that patch makes very explicit which JSFunction.object are not necessary. Here is a result of using a browser for a while:


**********************************
     new   marked    freed   total

     917      917        0      917
     555     1472        0     1472
     347     1819        0     1819
     360     2179        0     2179
     420     2597        2     2599
after startup
       0     2597        0     2599
      24     2621        0     2623
     142     2763        0     2765
       0     2763        0     2765
       0     2763        0     2765
       0     2763        0     2765
       0     2763        0     2765
       0     2763        0     2765
       0     2763        0     2765
using gmail       0     2763        0     2765
       0     2763        0     2765
      10     2773        0     2775
      86     2859        0     2861
       0     2859        0     2861
      20     2859       20     2881
      21     2880        0     2902
       0     2880        0     2902
Yes, the faulty API design decisions, which go back to 1996 or really earlier, and which precede sharing precompiled scripts and ECMA-compatible function definition processing, should be removed for those embeddings who can opt in. Typically we avoid #ifdefs (since JS_THREADSAFE at least) and prefer runtime opt-in. In this case the options would be embedding-global, not per-runtime or per-context.

Another idea is to do what we did with JS_GetStringBytes when Unicode went in (even then we didn't want to change API signatures): make JS_GetFunctionObject suppress OOM error (which is a hard case anyway) by returning an error object (preallocated). This smells bad, but it might beat the runtime embedding option.

/be
(Assignee)

Updated

11 years ago
Depends on: 410400
(Assignee)

Updated

11 years ago
Depends on: 413345
(Assignee)

Updated

10 years ago
Depends on: 424376
(Assignee)

Comment 3

10 years ago
This optimization became irrelevant in the view of bug 423874.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.