Bug 109368 – Shrink-wrap UnlinkedCodeBlock members.

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Andreas Kling]

There's a small opportunity for some $aving$ here.

[Andreas Kling]

Created attachment 187459 [details]
Patch

[WebKit Review Bot]

Attachment 187459 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h']" exit_code: 1
Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Oliver Hunt]

Comment on attachment 187459 [details]
Patch

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

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:511
> +    unsigned m_resolveOperationCount;
> +    unsigned m_putToBaseOperationCount;
> +    unsigned m_arrayProfileCount;
> +    unsigned m_arrayAllocationProfileCount;
> +    unsigned m_objectAllocationProfileCount;
> +    unsigned m_valueProfileCount;
> +    unsigned m_llintCallLinkInfoCount;

I think if you drop these down to 27 bits each you can shave 4 bytes off - would this save us anything?

[Andreas Kling]

(In reply to comment #3)
> (From update of attachment 187459 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187459&action=review
> 
> > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:511
> > +    unsigned m_resolveOperationCount;
> > +    unsigned m_putToBaseOperationCount;
> > +    unsigned m_arrayProfileCount;
> > +    unsigned m_arrayAllocationProfileCount;
> > +    unsigned m_objectAllocationProfileCount;
> > +    unsigned m_valueProfileCount;
> > +    unsigned m_llintCallLinkInfoCount;
> 
> I think if you drop these down to 27 bits each you can shave 4 bytes off - would this save us anything?

We'd need another 4 bytes to make a difference. :/
There's ways we can do that of course, but nothing as trivial as this game of musical chairs.

[Andreas Kling]

Committed r142387: <http://trac.webkit.org/changeset/142387>

[Oliver Hunt]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 187459 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=187459&action=review
> > 
> > > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:511
> > > +    unsigned m_resolveOperationCount;
> > > +    unsigned m_putToBaseOperationCount;
> > > +    unsigned m_arrayProfileCount;
> > > +    unsigned m_arrayAllocationProfileCount;
> > > +    unsigned m_objectAllocationProfileCount;
> > > +    unsigned m_valueProfileCount;
> > > +    unsigned m_llintCallLinkInfoCount;
> > 
> > I think if you drop these down to 27 bits each you can shave 4 bytes off - would this save us anything?
> 
> We'd need another 4 bytes to make a difference. :/
> There's ways we can do that of course, but nothing as trivial as this game of musical chairs.

i don't think it is strictly safe, but see what happens if you drop these down to 20 bits each - that should be more than enough to get through membuster, and will tell us if there's anything to be gained by work to reduce the size some more

[Andreas Kling]

(In reply to comment #6)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (From update of attachment 187459 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=187459&action=review
> > > 
> > > > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:511
> > > > +    unsigned m_resolveOperationCount;
> > > > +    unsigned m_putToBaseOperationCount;
> > > > +    unsigned m_arrayProfileCount;
> > > > +    unsigned m_arrayAllocationProfileCount;
> > > > +    unsigned m_objectAllocationProfileCount;
> > > > +    unsigned m_valueProfileCount;
> > > > +    unsigned m_llintCallLinkInfoCount;
> > > 
> > > I think if you drop these down to 27 bits each you can shave 4 bytes off - would this save us anything?
> > 
> > We'd need another 4 bytes to make a difference. :/
> > There's ways we can do that of course, but nothing as trivial as this game of musical chairs.
> 
> i don't think it is strictly safe, but see what happens if you drop these down to 20 bits each - that should be more than enough to get through membuster, and will tell us if there's anything to be gained by work to reduce the size some more

There's a synthetic memory pressure signal sent at the end of Membuster, which causes WebKit to discard all compiled code. Because of this, any reduction in UnlinkedCodeBlock size really only helps the peak number, not the end number. That's why I'm hesitant to do anything too crazy here.

For structures that persist the low-memory signal, there's more desperate things we can do, favoring fixed-size arrays over Vector if the data never changes, stuff like that.

[Roger Fong]

Right after this patch landed all of the layout tests and the jscore tests on windows started crashing/failing.

I'll get a stack trace and post it here but feel free to roll it out as well.

[Roger Fong]

Assertion failure: ASSERT(m_jitCodeForCallWithArityCheck) in Executable.h:generatedJitCodeForCallWithArityCheck();

Here's the stack trace:
>	JavaScriptCore.dll!JSC::ExecutableBase::generatedJITCodeForCallWithArityCheck()  Line 142 + 0x38 bytes	C++
 	JavaScriptCore.dll!JSC::ExecutableBase::generatedJITCodeWithArityCheckFor(JSC::CodeSpecializationKind kind=CodeForCall)  Line 156 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::lazyLinkFor(JSC::ExecState * callFrame=0x084a00c8, JSC::CodeSpecializationKind kind=CodeForCall)  Line 2146 + 0x10 bytes	C++
 	JavaScriptCore.dll!cti_vm_lazyLinkCall(void * * args=0x0030d674)  Line 2164 + 0xb bytes	C++
 	JavaScriptCore.dll!@cti_op_create_this@4()  + 0x17f bytes	C++
 	JavaScriptCore.dll!JSC::JITCode::execute(JSC::JSStack * stack=0x01f0d528, JSC::ExecState * callFrame=0x084a0058, JSC::JSGlobalData * globalData=0x01f154f8)  Line 135 + 0x29 bytes	C++
 	JavaScriptCore.dll!JSC::Interpreter::execute(JSC::ProgramExecutable * program=0x0923ed78, JSC::ExecState * callFrame=0x0406f5a0, JSC::JSObject * thisObj=0x03afff78)  Line 983 + 0x28 bytes	C++
 	JavaScriptCore.dll!JSC::evaluate(JSC::ExecState * exec=0x0406f5a0, const JSC::SourceCode & source={...}, JSC::JSValue thisValue={...}, JSC::JSValue * returnedException=0x0030e2f4)  Line 77	C++
 	WebKit.dll!WebCore::JSMainThreadExecState::evaluate(JSC::ExecState * exec=0x0406f5a0, const JSC::SourceCode & source={...}, JSC::JSValue thisValue={...}, JSC::JSValue * exception=0x0030e2f4)  Line 77 + 0x1d bytes	C++
 	WebKit.dll!WebCore::ScriptController::evaluateInWorld(const WebCore::ScriptSourceCode & sourceCode={...}, WebCore::DOMWrapperWorld * world=0x01f0d9d8)  Line 141 + 0x23 bytes	C++
 	WebKit.dll!WebCore::ScriptController::evaluate(const WebCore::ScriptSourceCode & sourceCode={...})  Line 158 + 0x16 bytes	C++
 	WebKit.dll!WebCore::ScriptElement::executeScript(const WebCore::ScriptSourceCode & sourceCode={...})  Line 304 + 0x17 bytes	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent(WebCore::PendingScript & pendingScript={...})  Line 144	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeParsingBlockingScript()  Line 120	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeParsingBlockingScripts()  Line 194 + 0x8 bytes	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad(WebCore::CachedResource * cachedScript=0x0892a2c8)  Line 204	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource * cachedResource=0x0892a2c8)  Line 794	C++
 	WebKit.dll!WebCore::CachedResource::checkNotify()  Line 378 + 0x13 bytes	C++
 	WebKit.dll!WebCore::CachedScript::data(WTF::PassRefPtr<WebCore::ResourceBuffer> data={...}, bool allDataReceived=true)  Line 90 + 0xf bytes	C++
 	WebKit.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime=0.00000000000000000)  Line 279	C++
 	WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x089b1b58, double finishTime=0.00000000000000000)  Line 457 + 0x18 bytes	C++
 	WebKit.dll!WebCore::didFinishLoading(_CFURLConnection * conn=0x08a378e0, const void * clientInfo=0x089b1b58)  Line 298 + 0x26 bytes	C++
 	CFNetwork.dll!URLConnectionClient::_clientDidFinishLoading(URLConnectionClient::ClientConnectionEventQueue * preQ=0x0030e728)  Line 1739 + 0x2b bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x0030e8e4, long count=4)  Line 2256	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x090609dc, long count=4)  Line 2328 + 0x19 bytes	C++
 	CFNetwork.dll!URLConnectionClient::processEvents()  Line 360 + 0x21 bytes	C++
 	CFNetwork.dll!URLConnectionWndProc(HWND__ * hWnd=0x00150336, unsigned int message=1231, unsigned int wParam=144931040, long lParam=0)  Line 109	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x23 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchMessageWorker@8()  + 0xed bytes	
 	user32.dll!_DispatchMessageW@4()  + 0xf bytes	
 	DumpRenderTree.dll!runTest(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & inputLine="C:\cygwin\home\buildbot\WebKit\OpenSource\LayoutTests\http\tests\inspector\network\network-preview-json.html")  Line 1050 + 0xf bytes	C++
 	DumpRenderTree.dll!dllLauncherEntryPoint(int argc=2, const char * * argv=0x01d11c10)  Line 1437 + 0x33 bytes	C++
 	DumpRenderTree.exe!main(int argc=2, const char * * argv=0x01d11c10)  Line 206 + 0x10 bytes	C++
 	DumpRenderTree.exe!__tmainCRTStartup()  Line 597 + 0x17 bytes	C
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes

[Andreas Kling]

Old patch, doesn't apply anymore, etc.