Bug 132360 - Don't hold on to BindingNodes forever
Summary: Don't hold on to BindingNodes forever
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on: 132431
Blocks:
  Show dependency tree / graph
 
Reported: 2014-04-29 15:11 PDT by Oliver Hunt
Modified: 2014-05-01 10:24 PDT (History)
1 user (show)

See Also:

Attachments
Patch (24.08 KB, patch)
2014-04-29 15:15 PDT, Oliver Hunt 		ggaren: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2014-04-29 15:11:43 PDT 
Don't hold on to BindingNodes forever
Comment 1 Oliver Hunt 2014-04-29 15:15:37 PDT 
Created attachment 230429 [details]
Patch
Comment 2 Geoffrey Garen 2014-04-29 15:20:44 PDT 
Comment on attachment 230429 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230429&action=review

r=me

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:196
> +    // We make an isolated copy of the parameter string as we don't want to keep the
> +    // full source string alive.
> +    String parameterString = source.provider()->getRange(node->parametersStartOffset(), node->parametersEndOffset()).isolatedCopy();

An executable always keeps its full source provider alive, so I don't think isolatedCopy() is an improvement here.
Comment 3 Oliver Hunt 2014-04-29 15:21:24 PDT 
Comment on attachment 230429 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230429&action=review

>> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:196
>> +    String parameterString = source.provider()->getRange(node->parametersStartOffset(), node->parametersEndOffset()).isolatedCopy();
> 
> An executable always keeps its full source provider alive, so I don't think isolatedCopy() is an improvement here.

unlinked executables don't :D
Comment 4 Oliver Hunt 2014-04-29 15:23:47 PDT 
Committed r167964: <http://trac.webkit.org/changeset/167964>
Comment 5 WebKit Commit Bot 2014-05-01 10:24:00 PDT 
Re-opened since this is blocked by bug 132431