Last Comment Bug 779975 - ref count ScriptSource
: ref count ScriptSource
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: :Benjamin Peterson
:
:
Mentors:
Depends on:
Blocks: 776741
  Show dependency treegraph
 
Reported: 2012-08-02 13:47 PDT by :Benjamin Peterson
Modified: 2012-08-07 07:38 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
implementing ref counting (18.30 KB, patch)
2012-08-02 13:49 PDT, :Benjamin Peterson
jorendorff: review+
Details | Diff | Splinter Review

Description :Benjamin Peterson 2012-08-02 13:47:43 PDT
Right now, ScriptSource is hacked onto the GC. This creates icky things like attachToRuntime and the IGC hacks in JSScript::setScriptSource. Since ScriptSource doesn't contain any structs, it would be easier to reference count script sources. Reference counting would also free the source as soon as the last script owning it disappeared instead of waiting for full GC.
Comment 1 :Benjamin Peterson 2012-08-02 13:49:06 PDT
Created attachment 648471 [details] [diff] [review]
implementing ref counting

The only slightly tricky part is that MemoryMetrics has to use a HashSet to avoid counting the same source multiple times.
Comment 2 Jason Orendorff [:jorendorff] 2012-08-03 11:24:10 PDT
Comment on attachment 648471 [details] [diff] [review]
implementing ref counting

Review of attachment 648471 [details] [diff] [review]:
-----------------------------------------------------------------

Wonderful. This is so much better than the GC hack.

My experience is, anything like this eventually gets turned into a real GC-thing; but if that happens in time, fine, and there's no rush.

::: js/src/jsscript.cpp
@@ +1293,2 @@
>      JS_ASSERT(ready());
>      // data is a union, but both members are pointers to allocated memory, so

Pre-existing style nit: while you're in here, please put a blank line before this comment (since it is not at the top of a block)
Comment 4 Ed Morley [:emorley] 2012-08-07 07:38:32 PDT
https://hg.mozilla.org/mozilla-central/rev/fa77c8c2a346

Note You need to log in before you can comment on or make changes to this bug.