109368 2013-02-09 20:49:50 -0800 Shrink-wrap UnlinkedCodeBlock members. 2013-09-06 18:44:38 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX Performance P2 Normal --- 109601 1 kling kling kling oliver roger_fong webkit.review.bot oldest_to_newest 829176 0 kling 2013-02-09 20:49:50 -0800 There's a small opportunity for some $aving$ here. 829177 1 187459 kling 2013-02-09 20:50:21 -0800 Created attachment 187459 Patch 829179 2 webkit.review.bot 2013-02-09 20:57:11 -0800 Attachment 187459 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. 829188 3 187459 oliver 2013-02-09 21:14:24 -0800 Comment on attachment 187459 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? 829208 4 kling 2013-02-09 22:41:27 -0800 (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. 829209 5 kling 2013-02-09 22:43:25 -0800 Committed r142387: <http://trac.webkit.org/changeset/142387> 829318 6 oliver 2013-02-10 10:56:05 -0800 (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 830256 7 kling 2013-02-11 15:52:33 -0800 (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. 831109 8 roger_fong 2013-02-12 11:23:10 -0800 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. 831117 9 roger_fong 2013-02-12 11:35:19 -0800 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 925957 10 kling 2013-09-06 18:44:38 -0700 Old patch, doesn't apply anymore, etc. 187459 2013-02-09 20:50:21 -0800 2013-02-09 21:14:24 -0800 Patch bug-109368.diff text/plain 3775 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDdkZjY3ZDMuLmFiMDZlMTAgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTAyLTA5ICBBbmRyZWFzIEtsaW5n ICA8YWtsaW5nQGFwcGxlLmNvbT4KKworICAgICAgICBTaHJpbmstd3JhcCBVbmxpbmtlZENvZGVC bG9jayBtZW1iZXJzLgorICAgICAgICA8aHR0cDovL3dlYmtpdC5vcmcvMTA5MzY4PgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJlYXJyYW5nZSB0aGUg bWVtYmVycyBvZiBVbmxpbmtlZENvZGVCbG9jayB0byBhdm9pZCB1bm5lY2Vzc2FyeSBwYWRkaW5n IG9uIDY0LWJpdC4KKyAgICAgICAgS25vY2tzIH42MDAgS0Igb2ZmIG9mIHRoZSBNZW1idXN0ZXIz IHBlYWsuCisKKyAgICAgICAgKiBieXRlY29kZS9VbmxpbmtlZENvZGVCbG9jay5jcHA6CisgICAg ICAgIChKU0M6OlVubGlua2VkQ29kZUJsb2NrOjpVbmxpbmtlZENvZGVCbG9jayk6CisgICAgICAg ICogYnl0ZWNvZGUvVW5saW5rZWRDb2RlQmxvY2suaDoKKyAgICAgICAgKFVubGlua2VkQ29kZUJs b2NrKToKKwogMjAxMy0wMi0wOCAgR2F2aW4gQmFycmFjbG91Z2ggIDxiYXJyYWNsb3VnaEBhcHBs ZS5jb20+CiAKICAgICAgICAgT2JqZWN0aXZlLUMgQVBJIGZvciBKYXZhU2NyaXB0Q29yZQpkaWZm IC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL1VubGlua2VkQ29kZUJsb2Nr LmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29kZS9VbmxpbmtlZENvZGVCbG9jay5j cHAKaW5kZXggODQ2MmI5YS4uYTNhNGRkMyAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRD b3JlL2J5dGVjb2RlL1VubGlua2VkQ29kZUJsb2NrLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvYnl0ZWNvZGUvVW5saW5rZWRDb2RlQmxvY2suY3BwCkBAIC0xNjAsNyArMTYwLDYgQEAg VW5saW5rZWRDb2RlQmxvY2s6OlVubGlua2VkQ29kZUJsb2NrKEpTR2xvYmFsRGF0YSogZ2xvYmFs RGF0YSwgU3RydWN0dXJlKiBzdHJ1Y3QKICAgICAsIG1fbnVtVmFycygwKQogICAgICwgbV9udW1D YWxsZWVSZWdpc3RlcnMoMCkKICAgICAsIG1fbnVtUGFyYW1ldGVycygwKQotICAgICwgbV9nbG9i YWxEYXRhKGdsb2JhbERhdGEpCiAgICAgLCBtX2FyZ3VtZW50c1JlZ2lzdGVyKC0xKQogICAgICwg bV9uZWVkc0Z1bGxTY29wZUNoYWluKGluZm8ubV9uZWVkc0FjdGl2YXRpb24pCiAgICAgLCBtX3Vz ZXNFdmFsKGluZm8ubV91c2VzRXZhbCkKQEAgLTE2OCwxMCArMTY3LDEwIEBAIFVubGlua2VkQ29k ZUJsb2NrOjpVbmxpbmtlZENvZGVCbG9jayhKU0dsb2JhbERhdGEqIGdsb2JhbERhdGEsIFN0cnVj dHVyZSogc3RydWN0CiAgICAgLCBtX2lzU3RyaWN0TW9kZShpbmZvLm1faXNTdHJpY3RNb2RlKQog ICAgICwgbV9pc0NvbnN0cnVjdG9yKGluZm8ubV9pc0NvbnN0cnVjdG9yKQogICAgICwgbV9oYXND YXB0dXJlZFZhcmlhYmxlcyhmYWxzZSkKKyAgICAsIG1fY29kZVR5cGUoY29kZVR5cGUpCisgICAg LCBtX2ZlYXR1cmVzKDApCiAgICAgLCBtX2ZpcnN0TGluZSgwKQogICAgICwgbV9saW5lQ291bnQo MCkKLSAgICAsIG1fZmVhdHVyZXMoMCkKLSAgICAsIG1fY29kZVR5cGUoY29kZVR5cGUpCiAgICAg LCBtX3Jlc29sdmVPcGVyYXRpb25Db3VudCgwKQogICAgICwgbV9wdXRUb0Jhc2VPcGVyYXRpb25D b3VudCgxKQogICAgICwgbV9hcnJheVByb2ZpbGVDb3VudCgwKQpAQCAtMTc5LDYgKzE3OCw3IEBA IFVubGlua2VkQ29kZUJsb2NrOjpVbmxpbmtlZENvZGVCbG9jayhKU0dsb2JhbERhdGEqIGdsb2Jh bERhdGEsIFN0cnVjdHVyZSogc3RydWN0CiAgICAgLCBtX29iamVjdEFsbG9jYXRpb25Qcm9maWxl Q291bnQoMCkKICAgICAsIG1fdmFsdWVQcm9maWxlQ291bnQoMCkKICAgICAsIG1fbGxpbnRDYWxs TGlua0luZm9Db3VudCgwKQorICAgICwgbV9nbG9iYWxEYXRhKGdsb2JhbERhdGEpCiAjaWYgRU5B QkxFKEJZVEVDT0RFX0NPTU1FTlRTKQogICAgICwgbV9ieXRlY29kZUNvbW1lbnRJdGVyYXRvcigw KQogI2VuZGlmCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYnl0ZWNvZGUvVW5s aW5rZWRDb2RlQmxvY2suaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29kZS9Vbmxpbmtl ZENvZGVCbG9jay5oCmluZGV4IDNjYTI3YWUuLmRhZDlmZDUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9ieXRlY29kZS9VbmxpbmtlZENvZGVCbG9jay5oCisrKyBiL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9ieXRlY29kZS9VbmxpbmtlZENvZGVCbG9jay5oCkBAIC00ODYsOCArNDg2 LDYgQEAgcHJpdmF0ZToKICAgICBSZWZDb3VudGVkQXJyYXk8VW5saW5rZWRJbnN0cnVjdGlvbj4g bV91bmxpbmtlZEluc3RydWN0aW9uczsKIAogICAgIGludCBtX251bVBhcmFtZXRlcnM7Ci0gICAg SlNHbG9iYWxEYXRhKiBtX2dsb2JhbERhdGE7Ci0KICAgICBpbnQgbV90aGlzUmVnaXN0ZXI7CiAg ICAgaW50IG1fYXJndW1lbnRzUmVnaXN0ZXI7CiAgICAgaW50IG1fYWN0aXZhdGlvblJlZ2lzdGVy OwpAQCAtNDk4LDExICs0OTYsMjEgQEAgcHJpdmF0ZToKICAgICBib29sIG1faXNTdHJpY3RNb2Rl IDogMTsKICAgICBib29sIG1faXNDb25zdHJ1Y3RvciA6IDE7CiAgICAgYm9vbCBtX2hhc0NhcHR1 cmVkVmFyaWFibGVzIDogMTsKKyAgICBDb2RlVHlwZSBtX2NvZGVUeXBlIDogMjsKKyAgICBDb2Rl RmVhdHVyZXMgbV9mZWF0dXJlcyA6IDE2OworCiAgICAgdW5zaWduZWQgbV9maXJzdExpbmU7CiAg ICAgdW5zaWduZWQgbV9saW5lQ291bnQ7CiAKLSAgICBDb2RlRmVhdHVyZXMgbV9mZWF0dXJlczsK LSAgICBDb2RlVHlwZSBtX2NvZGVUeXBlOworICAgIHVuc2lnbmVkIG1fcmVzb2x2ZU9wZXJhdGlv bkNvdW50OworICAgIHVuc2lnbmVkIG1fcHV0VG9CYXNlT3BlcmF0aW9uQ291bnQ7CisgICAgdW5z aWduZWQgbV9hcnJheVByb2ZpbGVDb3VudDsKKyAgICB1bnNpZ25lZCBtX2FycmF5QWxsb2NhdGlv blByb2ZpbGVDb3VudDsKKyAgICB1bnNpZ25lZCBtX29iamVjdEFsbG9jYXRpb25Qcm9maWxlQ291 bnQ7CisgICAgdW5zaWduZWQgbV92YWx1ZVByb2ZpbGVDb3VudDsKKyAgICB1bnNpZ25lZCBtX2xs aW50Q2FsbExpbmtJbmZvQ291bnQ7CisKKyAgICBKU0dsb2JhbERhdGEqIG1fZ2xvYmFsRGF0YTsK IAogICAgIFZlY3Rvcjx1bnNpZ25lZD4gbV9qdW1wVGFyZ2V0czsKIApAQCAtNTI0LDE0ICs1MzIs NiBAQCBwcml2YXRlOgogICAgIHNpemVfdCBtX2J5dGVjb2RlQ29tbWVudEl0ZXJhdG9yOwogI2Vu ZGlmCiAKLSAgICB1bnNpZ25lZCBtX3Jlc29sdmVPcGVyYXRpb25Db3VudDsKLSAgICB1bnNpZ25l ZCBtX3B1dFRvQmFzZU9wZXJhdGlvbkNvdW50OwotICAgIHVuc2lnbmVkIG1fYXJyYXlQcm9maWxl Q291bnQ7Ci0gICAgdW5zaWduZWQgbV9hcnJheUFsbG9jYXRpb25Qcm9maWxlQ291bnQ7Ci0gICAg dW5zaWduZWQgbV9vYmplY3RBbGxvY2F0aW9uUHJvZmlsZUNvdW50OwotICAgIHVuc2lnbmVkIG1f dmFsdWVQcm9maWxlQ291bnQ7Ci0gICAgdW5zaWduZWQgbV9sbGludENhbGxMaW5rSW5mb0NvdW50 OwotCiBwdWJsaWM6CiAgICAgc3RydWN0IFJhcmVEYXRhIHsKICAgICAgICAgV1RGX01BS0VfRkFT VF9BTExPQ0FURUQ7Cg==