forked from WebKit/WebKit-http
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specialize args prototype #1
Open
caiolima
wants to merge
41
commits into
master
Choose a base branch
from
specialize-args-prototype
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t into specialize-args-prototype
…t into specialize-args-prototype
…t into specialize-args-prototype
…t into specialize-args-prototype
caiolima
pushed a commit
that referenced
this pull request
May 28, 2017
https://bugs.webkit.org/show_bug.cgi?id=172288 Patch by Romain Bellessort <romain.bellessort@crf.canon.fr> on 2017-05-23 Reviewed by Chris Dumez. Two changes are implemented in this patch: - Change #1: An issue was reported to GH [1] while working on respondInClosedState implementation. This issue has now been fixed, and this patch aligns implementation with spec [2]. - Change #2: In addition, this patch also fixes a bug that went unnoticed as code is not yet reachable (usage of controller.@reader is not valid and is therefore replaced by controller.@controlledReadableStream.@reader). [1] whatwg/streams#686 [2] https://streams.spec.whatwg.org/#readable-byte-stream-controller-respond-in-closed-state No added test as: - Change #1 does not change behavior; - Change #2 is not testable as the code is not yet reachable. * Modules/streams/ReadableByteStreamInternals.js: (readableByteStreamControllerRespondInClosedState): Aligned with spec. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@217279 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
force-pushed
the
specialize-args-prototype
branch
from
August 20, 2017 15:12
2084cfe
to
088e603
Compare
caiolima
pushed a commit
that referenced
this pull request
Apr 23, 2018
…eady has focus is a confusing experience https://bugs.webkit.org/show_bug.cgi?id=184635 <rdar://problem/39440642> Reviewed by Tim Horton. Source/WebKit: Currently on iOS, we allow element focus to present UI if the keyboard is already shown. In extra zoom mode, this would lead to a confusing experience when the focus form control overlay is disabled, since fullscreen input view controllers are swapped out from underneath the user. Currently, this also puts the UI process into a bad state where the focused form control overlay is active, but still hidden. This patch makes some tweaks to input view controller handling in the UI process to address these issues, and also adds WebKitTestRunner support for simulating interactions with select menus in extra zoom mode. See comments below for more detail. Test: fast/events/extrazoom/change-focus-during-change-event.html * UIProcess/API/Cocoa/WKUIDelegatePrivate.h: Add new SPI delegate hooks to notify the UI delegate when view controllers are presented or dismissed in extra zoom mode. See -presentViewControllerForCurrentAssistedNode and -dismissAllInputViewControllers. * UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::takeBackgroundActivityTokenForFullscreenInput): (WebKit::WebProcessProxy::releaseBackgroundActivityTokenForFullscreenInput): See the comment below -dismissAllInputViewControllers. * UIProcess/WebProcessProxy.h: * UIProcess/ios/WKContentViewInteraction.mm: (-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]): In extra zoom mode, when changing focus from one assisted node to another, only allow the second node to be assisted if the focused form control overlay is being shown. Otherwise, (i.e. when a fullscreen input view controller is being presented), don't allow focus to start an input session. Additionally, make a minor tweak to allow the previous node to blur, even if we are not showing the keyboard for the new focused element. Without this adjustment, in the case where the page has programmatically focused another element while a fullscreen input view controller is presented, we'll show the old view controller for the new focused element. (-[WKContentView presentViewControllerForCurrentAssistedNode]): (-[WKContentView dismissAllInputViewControllers:]): Currently, when a fullscreen input view controller is presented, the web process gets backgrounded. This prevents event handlers from executing, which leads to strange behaviors in many cases (for instance: if we have a multiple select, and the "change" event handler blurs the select, the user may check or uncheck multiple items, but only the first change will actually take effect). To fix this, we maintain a background activity token while presenting an input view controller. (-[WKContentView focusedFormControlViewDidBeginEditing:]): Start hiding the focused form overlay when re-presenting an input view controller. This allows us to bail from showing fullscreen input UI for another focused element if focus programmatically changes while the current fullscreen input view controller is presented, due to the -isHidden check in -_startAssistingNode:. (-[WKContentView selectFormAccessoryPickerRow:]): Simulate tapping a given row in select menu UI in extra zoom mode. Tools: Add plumbing to support invoking `didHideKeyboardCallback` and `didShowKeyboardCallback` when (respectively) dismissing or presenting fullscreen input view controllers in extra zoom mode. * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm: (-[TestRunnerWKWebView initWithFrame:configuration:]): (-[TestRunnerWKWebView dealloc]): (-[TestRunnerWKWebView _invokeShowKeyboardCallbackIfNecessary]): (-[TestRunnerWKWebView _invokeHideKeyboardCallbackIfNecessary]): (-[TestRunnerWKWebView _keyboardDidShow:]): (-[TestRunnerWKWebView _keyboardDidHide:]): (-[TestRunnerWKWebView _webView:didPresentFocusedElementViewController:]): (-[TestRunnerWKWebView _webView:didDismissFocusedElementViewController:]): LayoutTests: Add a new layout test to exercise the following sequence of events in extra zoom mode: 1. Focus select element #1. 2. Choose an unselected option. 3. Programmatically focus select element #2 in the "change" event handler. 4. Choose an unselected option. 5. Programmatically blur select element #2 in the "change" event handler. * fast/events/extrazoom/change-focus-during-change-event-expected.txt: Added. * fast/events/extrazoom/change-focus-during-change-event.html: Added. * resources/ui-helper.js: (window.UIHelper.waitForKeyboardToHide.return.new.Promise): (window.UIHelper.waitForKeyboardToHide): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@230752 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Nov 13, 2018
…nd the marker item renderer. https://bugs.webkit.org/show_bug.cgi?id=191554 <rdar://problem/45825265> Reviewed by Antti Koivisto. Source/WebCore: While moving the marker item renderer to its correct subtree, we accidentally remove the soon-to-be parent anonymous block. Moving a renderer is a 2 step process: 1. Detach the renderer from its current parent 2. Attach it to its new parent. During step #1, we check if there is a chance to collapse anonymous blocks. In this case the soon-to-be-parent is a sibling anonymous block which, after detaching the marker sibling is not needed anymore (except we use it as the new parent). Test: fast/inline/marker-list-item-move-should-not-crash.html * rendering/updating/RenderTreeBuilder.cpp: (WebCore::RenderTreeBuilder::detach): * rendering/updating/RenderTreeBuilder.h: * rendering/updating/RenderTreeBuilderBlock.cpp: (WebCore::RenderTreeBuilder::Block::detach): * rendering/updating/RenderTreeBuilderBlock.h: * rendering/updating/RenderTreeBuilderList.cpp: (WebCore::RenderTreeBuilder::List::updateItemMarker): LayoutTests: * fast/inline/marker-list-item-move-should-not-crash-expected.txt: Added. * fast/inline/marker-list-item-move-should-not-crash.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238119 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Nov 14, 2018
…llFrame. https://bugs.webkit.org/show_bug.cgi?id=191579 <rdar://problem/45942472> Reviewed by Saam Barati. JSTests: * stress/regress-191579.js: Added. Source/JavaScriptCore: Both of these functions do a lot of work. It would be good for the topCallFrame to be correct should we need to throw an exception. For example, we've observed the following crash trace: * frame #0: WTFCrash() at Assertions.cpp:253 frame #1: ... frame #2: JSC::StructureIDTable::get(this=0x00006040000162f0, structureID=1874583248) at StructureIDTable.h:129 frame #3: JSC::VM::getStructure(this=0x0000604000016210, id=4022066896) at VM.h:705 frame #4: JSC::JSCell::structure(this=0x00007ffeefbbde30, vm=0x0000604000016210) const at JSCellInlines.h:125 frame #5: JSC::JSCell::classInfo(this=0x00007ffeefbbde30, vm=0x0000604000016210) const at JSCellInlines.h:335 frame #6: JSC::JSCell::inherits(this=0x00007ffeefbbde30, vm=0x0000604000016210, info=0x0000000105eaf020) const at JSCellInlines.h:302 frame WebKit#7: JSC::JSObject* JSC::jsCast<JSC::JSObject*, JSC::JSCell>(from=0x00007ffeefbbde30) at JSCast.h:36 frame WebKit#8: JSC::asObject(cell=0x00007ffeefbbde30) at JSObject.h:1299 frame WebKit#9: JSC::asObject(value=JSValue @ 0x00007ffeefbba380) at JSObject.h:1304 frame WebKit#10: JSC::Register::object(this=0x00007ffeefbbdd58) const at JSObject.h:1514 frame WebKit#11: JSC::ExecState::jsCallee(this=0x00007ffeefbbdd40) const at CallFrame.h:107 frame WebKit#12: JSC::ExecState::isStackOverflowFrame(this=0x00007ffeefbbdd40) const at CallFrameInlines.h:36 frame WebKit#13: JSC::StackVisitor::StackVisitor(this=0x00007ffeefbba860, startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800) at StackVisitor.cpp:52 frame WebKit#14: JSC::StackVisitor::StackVisitor(this=0x00007ffeefbba860, startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800) at StackVisitor.cpp:41 frame WebKit#15: void JSC::StackVisitor::visit<(JSC::StackVisitor::EmptyEntryFrameAction)0, JSC::Interpreter::getStackTrace(JSC::JSCell*, WTF::Vector<JSC::StackFrame, 0ul, WTF::CrashOnOverflow, 16ul>&, unsigned long, unsigned long)::$_3>(startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800, functor=0x00007ffeefbbaa60)::$_3 const&) at StackVisitor.h:147 frame WebKit#16: JSC::Interpreter::getStackTrace(this=0x0000602000005db0, owner=0x000062d00020cbe0, results=0x00006020000249d0, framesToSkip=0, maxStackSize=1) at Interpreter.cpp:437 frame WebKit#17: JSC::getStackTrace(exec=0x000062d00002c048, vm=0x0000631000000800, obj=0x000062d00020cbe0, useCurrentFrame=true) at Error.cpp:170 frame WebKit#18: JSC::ErrorInstance::finishCreation(this=0x000062d00020cbe0, exec=0x000062d00002c048, vm=0x0000631000000800, message=0x00007ffeefbbb800, useCurrentFrame=true) at ErrorInstance.cpp:119 frame WebKit#19: JSC::ErrorInstance::create(exec=0x000062d00002c048, vm=0x0000631000000800, structure=0x000062d0000f5730, message=0x00007ffeefbbb800, appender=0x0000000000000000, type=TypeNothing, useCurrentFrame=true)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred), JSC::RuntimeType, bool) at ErrorInstance.h:49 frame WebKit#20: JSC::createRangeError(exec=0x000062d00002c048, globalObject=0x000062d00002c000, message=0x00007ffeefbbb800, appender=0x0000000000000000)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred)) at Error.cpp:68 frame WebKit#21: JSC::createRangeError(exec=0x000062d00002c048, globalObject=0x000062d00002c000, message=0x00007ffeefbbb800) at Error.cpp:316 frame WebKit#22: JSC::createStackOverflowError(exec=0x000062d00002c048, globalObject=0x000062d00002c000) at ExceptionHelpers.cpp:77 frame WebKit#23: JSC::createStackOverflowError(exec=0x000062d00002c048) at ExceptionHelpers.cpp:72 frame WebKit#24: JSC::throwStackOverflowError(exec=0x000062d00002c048, scope=0x00007ffeefbbbaa0) at ExceptionHelpers.cpp:335 frame WebKit#25: JSC::ProxyObject::getOwnPropertySlotCommon(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbba80, slot=0x00007ffeefbbc720) at ProxyObject.cpp:372 frame WebKit#26: JSC::ProxyObject::getOwnPropertySlot(object=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbbd40, slot=0x00007ffeefbbc720) at ProxyObject.cpp:395 frame WebKit#27: JSC::JSObject::getNonIndexPropertySlot(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbbea0, slot=0x00007ffeefbbc720) at JSObjectInlines.h:150 frame WebKit#28: bool JSC::JSObject::getPropertySlot<false>(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbc320, slot=0x00007ffeefbbc720) at JSObject.h:1424 frame WebKit#29: JSC::JSObject::calculatedClassName(object=0x000062d000200e40) at JSObject.cpp:535 frame WebKit#30: JSC::Structure::toStructureShape(this=0x000062d000007410, value=JSValue @ 0x00007ffeefbbcae0, sawPolyProtoStructure=0x00007ffeefbbcf60) at Structure.cpp:1142 frame WebKit#31: JSC::TypeProfilerLog::processLogEntries(this=0x000060400000a950, reason=0x00007ffeefbbd5c0) at TypeProfilerLog.cpp:89 frame WebKit#32: JSC::JIT::doMainThreadPreparationBeforeCompile(this=0x0000619000034da0) at JIT.cpp:951 frame WebKit#33: JSC::JITWorklist::Plan::Plan(this=0x0000619000034d80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:43 frame WebKit#34: JSC::JITWorklist::Plan::Plan(this=0x0000619000034d80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:42 frame WebKit#35: JSC::JITWorklist::compileLater(this=0x0000616000001b80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:256 frame WebKit#36: JSC::LLInt::jitCompileAndSetHeuristics(codeBlock=0x000062d0001d88c0, exec=0x00007ffeefbbde30, loopOSREntryBytecodeOffset=0) at LLIntSlowPaths.cpp:391 frame WebKit#37: llint_replace(exec=0x00007ffeefbbde30, pc=0x00006040000161ba) at LLIntSlowPaths.cpp:516 frame WebKit#38: llint_entry at LowLevelInterpreter64.asm:98 frame #39: vmEntryToJavaScript at LowLevelInterpreter64.asm:296 ... This crash occurred because StackVisitor was seeing an invalid topCallFrame while trying to capture the Error stack while throwing a StackOverflowError below llint_replace. While in this specific example, it is questionable whether we should be executing JS code below TypeProfilerLog::processLogEntries(), it is correct to have set the topCallFrame in llint_replace. We do this by calling LLINT_BEGIN_NO_SET_PC() at the top of llint_replace. We also do the same for llint_osr. Note: both of these LLInt slow path functions are called with a fully initialized CallFrame. Hence, there's no issue with setting topCallFrame to their CallFrames for these functions. * llint/LLIntSlowPaths.cpp: (JSC::LLInt::LLINT_SLOW_PATH_DECL): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238141 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Dec 1, 2018
https://bugs.webkit.org/show_bug.cgi?id=192236 <rdar://problem/45792118> Reviewed by Geoffrey Garen. Source/WebCore: This patch introduces asynchronous content change observation. 1. Start observing synchronous content change and timer install as the result of dispatching mouseMoved event. 2. Start observing synchronous content change and style recalc schedule as the result of a timer callback (installed at #1). 3. Start observing synchronous content change as the result of a style recalc (scheduled at #2). This patch also extends the timeout value from 100ms to 250ms. Certain content prefer longer timeouts (see http://briancherne.github.io/jquery-hoverIntent/ for details). Test: fast/events/touch/ios/hover-when-style-change-is-async.html * dom/Document.cpp: (WebCore::Document::scheduleStyleRecalc): (WebCore::Document::updateStyleIfNeeded): * page/DOMTimer.cpp: (WebCore::DOMTimer::install): (WebCore::DOMTimer::fired): * platform/ios/wak/WKContentObservation.cpp: (WKStartObservingStyleRecalcScheduling): (WKStopObservingStyleRecalcScheduling): (WKIsObservingStyleRecalcScheduling): (WKSetShouldObserveNextStyleRecalc): (WKShouldObserveNextStyleRecalc): (WKSetObservedContentChange): * platform/ios/wak/WKContentObservation.h: LayoutTests: * fast/events/touch/ios/hover-when-style-change-is-async-expected.txt: Added. * fast/events/touch/ios/hover-when-style-change-is-async.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238759 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Mar 14, 2019
…ormats https://bugs.webkit.org/show_bug.cgi?id=195653 Reviewed by Mark Lam. JSTests: * stress/osr-entry-locals-none.js: Added. Source/JavaScriptCore: Let's consider the following graph. Block #0 ... 27:< 2:loc13> JSConstant(JS|UseAsOther, StringIdent, Strong:String (atomic) (identifier): , StructureID: 42679, bc#10, ExitValid) ... 28:< 2:loc13> ArithPow(DoubleRep:@437<Double>, Int32:@27, Double|UseAsOther, BytecodeDouble, Exits, bc#10, ExitValid) 29:<!0:-> MovHint(DoubleRep:@28<Double>, MustGen, loc7, W:SideState, ClobbersExit, bc#10, ExitValid) 30:< 1:-> SetLocal(DoubleRep:@28<Double>, loc7(M<Double>/FlushedDouble), machine:loc6, W:Stack(-8), bc#10, exit: bc#14, ExitValid) predicting BytecodeDouble ... 73:<!0:-> Jump(MustGen, T:#1, W:SideState, bc#71, ExitValid) Block #1 (bc#71): (OSR target) pred, #0 ... 102:<!2:loc15> GetLocal(Check:Untyped:@400, Double|MustGen|PureInt, BytecodeDouble, loc7(M<Double>/FlushedDouble), machine:loc6, R:Stack(-8), bc#120, ExitValid) predicting BytecodeDouble ... CFA at @28 says it is invalid since there are type contradiction (Int32:@27 v.s. StringIdent). So, of course, we do not propagate #0's type information to #1 since we become invalid state. However, #1 is still reachable since it is an OSR target. Since #0 was only the predecessor of #1, loc7's type information becomes None at the head of #1. Since loc7's AbstractValue is None, @102 GetLocal emits breakpoint. It is OK as long as OSR entry fails because AbstractValue validation requires the given value is None type. The issue here is that we skipped AbstractValue validation when we have FlushFormat information. Since loc7 has FlushedDouble format, DFG OSR entry code does not validate it against AbstractValue, which is None. Then, we hit the breakpoint emitted by @102. This patch performs AbstractValue validation against values even if we have FlushFormat. We should correctly configure AbstractValue for OSR entry's locals too to avoid unnecessary OSR entry failures in the future but anyway validating locals with AbstractValue is correct behavior here since DFGSpeculativeJIT relies on that. * dfg/DFGOSREntry.cpp: (JSC::DFG::prepareOSREntry): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@242841 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Mar 14, 2019
…s JITCode is singleton https://bugs.webkit.org/show_bug.cgi?id=195638 Reviewed by Mark Lam. This patch introduces a m_isShared flag to track whether the JITCode is shared between many CodeBlocks. This flag is used in `CodeBlock::setJITCode` and `CodeBlock::visitChildren` to avoid reporting duplicated extra memory for singleton JITCodes. With those changes, we now stop counting singleton LLIntEntrypoints as extra memory, since they are declared as static variables. This change can potentially avoid unecessary GC pressure, because extra memory is used by Heap::updateAllocationLimits() to update Heap limits. Even though it is hard to show performance difference for this change (see results below), it is important to keep extra memory usage correct. Otherwise, it can be a source of a complicated bug on GC in the future. Results from last run of Speedometer 2 comparing ToT and changes. We collected those numbers running Minibrowser on a MacBook Pro 15-inch with 2,6 GHz Intel Core i7. Both versions are with JIT disabled, since these singleton JITCode are only used by this configuration: Speedometer2 Run #1 ToT: 58.2 +- 1.1 changes: 57.9 +- 0.99 Speedometer2 Run #2 ToT: 58.5 +- 1.7 changes: 58.0 +- 1.5 Speedometer2 Run #2 ToT: 58.5 +- 0.99 changes: 57.1 +- 1.5 * bytecode/CodeBlock.cpp: (JSC::CodeBlock::estimatedSize): (JSC::CodeBlock::visitChildren): * bytecode/CodeBlock.h: (JSC::CodeBlock::setJITCode): * jit/JITCode.cpp: (JSC::JITCode::JITCode): (JSC::JITCodeWithCodeRef::JITCodeWithCodeRef): (JSC::DirectJITCode::DirectJITCode): (JSC::NativeJITCode::NativeJITCode): * jit/JITCode.h: (JSC::JITCode::isShared const): * llint/LLIntEntrypoint.cpp: (JSC::LLInt::setFunctionEntrypoint): (JSC::LLInt::setEvalEntrypoint): (JSC::LLInt::setProgramEntrypoint): (JSC::LLInt::setModuleProgramEntrypoint): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@242928 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Mar 19, 2019
https://bugs.webkit.org/show_bug.cgi?id=194648 Reviewed by Keith Miller. JSTests: * microbenchmarks/generate-multiple-llint-entrypoints.js: Added. Source/JavaScriptCore: 1. Making LLIntThunks singleton. Motivation: Former implementation has one LLIntThunk per type per VM. However, the generated code for every kind of thunk is essentially the same and we end up wasting memory (right now jitAllocationGranule = 32 bytes) when we have 2 or more VM instantiated. Turn these thunks into singleton will avoid such wasting. Tradeoff: This change comes with a price, because we will keep thunks allocated even when there is no VM instantiated. Considering WebCore use case, the situation of having no VM instantiated is uncommon, since once a VM is created through `commomVM()`, it will never be destroyed. Given that, this change does not impact the overall memory comsumption of WebCore/JSC. It also doesn't impact memory footprint, since thunks are generated lazily (see results below). Since we are keeping a static `MacroAssemblerCodeRef<JITThunkPtrTag>`, we have the assurance that JITed code will never be deallocated, given it is being pointed by `RefPtr<ExecutableMemoryHandle> m_executableMemory`. To understand why we decided to make LLIntThunks singleton instead of removing them, please see the comment on `llint/LLIntThunks.cpp`. 2. Making all LLIntEntrypoints singleton Motivation: With singleton LLIntThunks, we also can have singleton DirectJITCodes and NativeJITCodes for each LLIntEntrypoint type and avoid multiple allocations of objects with the same content. Tradeoff: As explained before, once we allocate an entrypoint, it will be alive until the program exits. However, the gains we can achieve in some use cases justifies such allocations. As DirectJITCode and NativeJITCode are ThreadSafeRefCounted and we are using `codeBlock->setJITCode(makeRef(*jitCode))`, their reference counter will never be less than 1. 3. Memory usage analysis This change reduces memory usage on stress/generate-multiple-llint-entrypoints.js by 2% and is neutral on JetStream 2. Following results were generated running each benchmark 6 times and using 95% Student's t distribution confidence interval. microbenchmarks/generate-multiple-llint-entrypoints.js (Changes uses less memory): Mean of memory peak on ToT: 122576896 bytes (confidence interval: 67747.2316) Mean of memory peak on Changes: 119248213.33 bytes (confidence interval: 50251.2718) JetStream2 (Neutral): Mean of memory peak on ToT: 5442742272 bytes (confidence interval: 134381565.9117) Mean of memory peak on Changes: 5384949760 bytes (confidence interval: 158413904.8352) 4. Performance Analysis This change is performance neutral on JetStream 2 and Speedometer 2. See results below.: JetStream 2 (Neutral): Mean of score on ToT: 139.58 (confidence interval: 2.44) Mean of score on Changes: 141.46 (confidence interval: 4.24) Speedometer run #1 ToT: 110 +- 2.9 Changes: 110 +- 1.8 Speedometer run #2 ToT: 110 +- 1.6 Changes: 108 +- 2.3 Speedometer run #3 ToT: 110 +- 3.0 Changes: 110 +- 1.4 * jit/JSInterfaceJIT.h: (JSC::JSInterfaceJIT::JSInterfaceJIT): * llint/LLIntEntrypoint.cpp: Here we are changing the usage or DirectJITCode by NativeJITCode on cases where there is no difference from address of calls with and without ArithCheck. (JSC::LLInt::setFunctionEntrypoint): (JSC::LLInt::setEvalEntrypoint): (JSC::LLInt::setProgramEntrypoint): (JSC::LLInt::setModuleProgramEntrypoint): (JSC::LLInt::setEntrypoint): * llint/LLIntEntrypoint.h: * llint/LLIntThunks.cpp: (JSC::LLInt::generateThunkWithJumpTo): (JSC::LLInt::functionForCallEntryThunk): (JSC::LLInt::functionForConstructEntryThunk): (JSC::LLInt::functionForCallArityCheckThunk): (JSC::LLInt::functionForConstructArityCheckThunk): (JSC::LLInt::evalEntryThunk): (JSC::LLInt::programEntryThunk): (JSC::LLInt::moduleProgramEntryThunk): (JSC::LLInt::functionForCallEntryThunkGenerator): Deleted. (JSC::LLInt::functionForConstructEntryThunkGenerator): Deleted. (JSC::LLInt::functionForCallArityCheckThunkGenerator): Deleted. (JSC::LLInt::functionForConstructArityCheckThunkGenerator): Deleted. (JSC::LLInt::evalEntryThunkGenerator): Deleted. (JSC::LLInt::programEntryThunkGenerator): Deleted. (JSC::LLInt::moduleProgramEntryThunkGenerator): Deleted. * llint/LLIntThunks.h: * runtime/ScriptExecutable.cpp: (JSC::setupLLInt): (JSC::ScriptExecutable::prepareForExecutionImpl): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@243136 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Apr 18, 2019
…flaky failure on Mac Release builds https://bugs.webkit.org/show_bug.cgi?id=196905 <rdar://problem/49886096> Reviewed by Tim Horton. This flaky test exercises a race condition between when attachment insertion updates are dispatched from the web process to the UI process, and when script is executed via -[WKWebView evaluateJavaScript:completionHandler:]. Since attachment insertion and removal updates from the web process to the UI process are scheduled on a zero- delay timer, we end up with this sequence of events in the problematic (failure) case: (a) [UI] Run script #1 (which calls `HTMLAttachmentElement.getAttachmentIdentifier`) ...IPC from UI to web process... (b) [Web] Evaluate script #1 in the web process, which schedules attachment updates on a zero-delay timer ...IPC from web to UI process... (c) [UI] Invoke completion handler for script #1 (d) [UI] Run script #2 (which calls `document.querySelector('img').attachmentIdentifier`) ...IPC from UI to web process... (e) [Web] Evaluate script #2 in the web process (f) [Web] Zero-delay timer fires and dispatches attachment updates to the UI process ...which means that script #2 will complete before the UI process has received the attachment updates sent in step (f). However, in the case where the flaky test succeeds, the zero-delay timer in (f) fires *before* script #2 is run in step (e). This patch fixes the flaky test by waiting until attachment insertion updates are guaranteed to be received in the UI process by waiting on a script message posted by the web process, after attachment updates are dispatched. * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm: (TestWebKitAPI::TEST): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244251 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Apr 22, 2019
https://bugs.webkit.org/show_bug.cgi?id=197118 <rdar://problem/49969960> Reviewed by Michael Saboff. JSTests: * stress/abstract-value-can-include-int52.js: Added. (foo): (index.index.8.index.60.index.65.index.1234.index.1234.parseInt.string_appeared_here.String.fromCharCode): Source/JavaScriptCore: Let's analyze this control flow diamond: #0 branch #1, #2 #1: PutStack(JSValue, loc42) Jump #3 #2: PutStack(Int52, loc42) Jump #3 #3: ... Our abstract value for loc42 at the head of #3 will contain an abstract value that us the union of Int52 with other things. Obviously in the above program, a GetStack for loc42 would be inavlid, since it might be loading either JSValue or Int52. However, the abstract interpreter just tracks what the value could be, and it could be Int52 or JSValue. When I did the Int52 refactoring, I expected such things to never happen, but it turns out it does. We should just allow for this instead of asserting against it since it's valid IR to do the above. * bytecode/SpeculatedType.cpp: (JSC::dumpSpeculation): * dfg/DFGAbstractValue.cpp: (JSC::DFG::AbstractValue::checkConsistency const): * dfg/DFGAbstractValue.h: (JSC::DFG::AbstractValue::validateTypeAcceptingBoxedInt52 const): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244480 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
May 8, 2019
https://bugs.webkit.org/show_bug.cgi?id=197496 Reviewed by Lucas Forschler. * BuildSlaveSupport/ews-app/ews/views/statusbubble.py: (StatusBubble._build_bubble): (StatusBubble._queue_position): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244930 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Aug 5, 2019
https://bugs.webkit.org/show_bug.cgi?id=200426 <rdar://problem/53912607> Reviewed by Antti Koivisto. The intrinsic width for a formatting root box has 2 sets of values now. One set(min/max) is stored in the established formatting context's state while the other is in the formatting context's state where the box lives. <div style="position: absolute"><div style="float: left; border: 1px solid green">foobar</div></div> The float box participates in the formatting context established by the absolutely position box, but it also establishes an inline formatting context. The min/max width pair in the established context is the width of the "foobar" (same value for min/max). This set is stored in the inline formatting state. However the float box has horizontal border so the "final" min/max width pair is expanded by this border value and stored in the formatting state where the box lives (which is different from the one it establishes). This and the "remove the formatting context type classes from the tree" changes open up interesting optimization opportunities. Here is a very simple case: <div style="display: inline-block; width: auto;"> <div style="float: left">some text</div> <div style="float: left">some super long .... text</div> <div></div> </div> In order to lay out this content properly, we 1. Compute the min/max width of the first float (expensive text measuring) 2. Compute the min/max width of the second float (some more expensive text measuring) 3. Compute the min/max width of the inline-block (that is pretty much the 2 float's min/max) 4. Lay out the 2 floats, the empty div and the inline-block using these min/max width pairs. Now if the inline-block box's display value is changed to "block" and the positioning is to absolute (style="display: box; position: absolute;") we currently(on trunk) tear down the render tree, build a new one and run all the steps again from #1 to #4. In LFC, we start with the following layout tree <container> -> block formatting context <container> -> inline formatting context <anonymous inline box> <container> -> inline formatting context <anonymous inline box> <container> -> inline formatting context and when the style change happens, we don't need to tear down the tree at all. Not only that, but since every formatting contexts stay the same we can just reuse their states and actually skip all the steps (even the positioning since the absolutely positioned container has static top/bottom/left/right). Surprisingly the final layout produces the exact same "display boxes" as the original layout. * layout/FormattingContext.h: (WebCore::Layout::FormattingContext::IntrinsicWidthConstraints::expand): * layout/FormattingContextGeometry.cpp: (WebCore::Layout::FormattingContext::Geometry::shrinkToFitWidth): * layout/FormattingState.h: (WebCore::Layout::FormattingState::setIntrinsicWidthConstraints): (WebCore::Layout::FormattingState::intrinsicWidthConstraints const): (WebCore::Layout::FormattingState::setIntrinsicWidthConstraintsForBox): (WebCore::Layout::FormattingState::clearIntrinsicWidthConstraints): (WebCore::Layout::FormattingState::intrinsicWidthConstraintsForBox const): * layout/blockformatting/BlockFormattingContext.cpp: (WebCore::Layout::BlockFormattingContext::computedIntrinsicWidthConstraints const): (WebCore::Layout::BlockFormattingContext::computeIntrinsicWidthConstraints const): Deleted. * layout/blockformatting/BlockFormattingContext.h: * layout/blockformatting/BlockFormattingContextGeometry.cpp: (WebCore::Layout::BlockFormattingContext::Geometry::intrinsicWidthConstraints): (WebCore::Layout::BlockFormattingContext::Geometry::intrinsicWidthConstraintsNeedChildrenWidth): Deleted. * layout/displaytree/DisplayBox.h: (WebCore::Display::Box::horizontalMarginBorderAndPadding const): * layout/inlineformatting/InlineFormattingContext.cpp: (WebCore::Layout::nextInPreOrder): (WebCore::Layout::InlineFormattingContext::computedIntrinsicWidthConstraints const): (WebCore::Layout::InlineFormattingContext::computeIntrinsicWidthForFormattingRoot const): (WebCore::Layout::InlineFormattingContext::computeIntrinsicWidthConstraints const): Deleted. (WebCore::Layout::InlineFormattingContext::computeIntrinsicWidthForFloatBox const): Deleted. (WebCore::Layout::InlineFormattingContext::computeIntrinsicWidthForInlineBlock const): Deleted. * layout/inlineformatting/InlineFormattingContext.h: * layout/tableformatting/TableFormattingContext.cpp: (WebCore::Layout::TableFormattingContext::computedIntrinsicWidthConstraints const): * layout/tableformatting/TableFormattingContext.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@248262 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Aug 31, 2019
…anches during interpretation https://bugs.webkit.org/show_bug.cgi?id=198650 Reviewed by Saam Barati. JSTests: * stress/object-allocation-sinking-interpretation-can-interpret-edges-that-can-be-proven-unreachable-in-ai.js: (main.v0): (main): Source/JavaScriptCore: Object Allocation Sinking phase has a lightweight abstract interpreter which interprets DFG nodes related to allocations and properties. This interpreter is lightweight since it does not track abstract values and conditions as deeply as AI does. It can happen that this interpreter interpret the control-flow edge that AI proved that is never taken. AI already knows some control-flow edges are never taken, and based on this information, AI can remove CheckStructure nodes. But ObjectAllocationSinking phase can trace this never-taken edges and propagate structure information that contradicts to the analysis done in ObjectAllocationSinking. Let's see the example. BB#0 35: NewObject([%AM:Object]) ... 47: Branch(ConstantTrue, T:#1, F:#2) BB#1 // This basic block is never taken due to @47's jump. ... 71: PutByOffset(@35, @66, id2{a}, 0, W:NamedProperties(2)) 72: PutStructure(@35, %AM:Object -> %Dx:Object, ID:60066) ... XX: Jump(#2) BB#2 ... 92: CheckStructure(@35, [%Dx:Object]) 93: PutByOffset(@35, @35, id2{a}, 0, W:NamedProperties(2)) ... AI removes @92 because AI knows BB#0 only takes BB#1 branch. @35's Structure is always %Dx so @92 is redundant. AI proved that @71 and @72 are always executed while BB#0 -> BB#2 edge is never taken so that @35 object's structure is proven at @92. After AI removes @92, ObjectAllocationSinking starts looking into this graph. BB#0 35: NewObject([%AM:Object]) ... 47: Branch(ConstantTrue, T:#1, F:#2) BB#1 // This basic block is never taken due to @47's jump. ... 71: PutByOffset(@35, @66, id2{a}, 0, W:NamedProperties(2)) 72: PutStructure(@35, %AM:Object -> %Dx:Object, ID:60066) ... XX: Jump(#2) BB#2 ... 93: PutByOffset(@35, @35, id2{a}, 0, W:NamedProperties(2)) ... YY: Jump(#3) BB#3 ... ZZ: <HERE> want to materialize @35's sunk object. Since AI does not change the @47 Branch to Jump (it is OK anyway), BB#0 -> BB#2 edge remains and ObjectAllocationSinking phase propagates information in BB#0's %AM structure information to BB#2. ObjectAllocationSinking phase converts @35 to PhantomNewObject, removes PutByOffset and PutStructure, and insert MaterializeNewObject in @zz. At this point, ObjectAllocationSinking lightweight interpreter gets two structures while AI gets one: @35's original one (%AM) and @72's replaced one (%Dx). Since AI already proved @zz only gets %Dx, AI removed @92 CheckStructure. But this is not known to ObjectAllocationSinking phase's interpretation. So when creating recovery data, MultiPutByOffset includes two structures, %AM and %Dx. This is OK since MultiPutByOffset takes conservative set of structures and performs switching. But the problem here is that %AM's id2{a} offset is -1 since %AM does not have such a property. So when creating MultiPutByOffset in ObjectAllocationSinking, we accidentally create MultiPutByOffset with -1 offset data, and lowering phase hits the debug assertion. 187: MultiPutByOffset(@138, @138, id2{a}, <Replace: [%AM:Object], offset = -1, >, <Replace: [%Dx:Object], offset = 0, >) This bug is harmless since %AM structure comparison never meets at runtime. But we are not considering the case including `-1` offset property in MultiPutByOffset data. In this patch, we just filter out apparently wrong structures when creating MultiPutByOffset in ObjectAllocationSinking. This is OK since it never comes at runtime. * dfg/DFGObjectAllocationSinkingPhase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@249306 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Nov 12, 2019
…oaded with webkit-patch --no-review https://bugs.webkit.org/show_bug.cgi?id=203903 Reviewed by David Kilzer. * BuildSlaveSupport/ews-app/ews/fetcher.py: (BugzillaPatchFetcher.fetch): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@252150 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Jan 7, 2020
…wind and lldb. https://bugs.webkit.org/show_bug.cgi?id=205050 Reviewed by Michael Saboff. Before this patch, the stack trace from inside a probe function is cut off at ctiMasmProbeTrampoline: (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) ... frame #4: 0x0000000100824607 JavaScriptCore`WTF::Function<void (JSC::Probe::Context&)>::operator(this=0x000000010b88bd00, in=0x00007ffeefbfd400)(JSC::Probe::Context&) const at Function.h:79:35 frame #5: 0x0000000100823996 JavaScriptCore`JSC::stdFunctionCallback(context=0x00007ffeefbfd400) at MacroAssembler.cpp:53:5 frame #6: 0x000000010082701e JavaScriptCore`JSC::Probe::executeProbe(state=0x00007ffeefbfd480) at ProbeContext.cpp:51:5 frame WebKit#7: 0x000000010082614b JavaScriptCore`ctiMasmProbeTrampoline + 299 (lldb) After this patch, we'll now get the full stack trace from inside the probe function: (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) ... frame #4: 0x0000000100826d17 JavaScriptCore`WTF::Function<void (JSC::Probe::Context&)>::operator(this=0x0000000106b878f8, in=0x00007ffeefbfd400)(JSC::Probe::Context&) const at Function.h:79:35 frame #5: 0x0000000100826106 JavaScriptCore`JSC::stdFunctionCallback(context=0x00007ffeefbfd400) at MacroAssembler.cpp:53:5 frame #6: 0x000000010082986e JavaScriptCore`JSC::Probe::executeProbe(state=0x00007ffeefbfd480) at ProbeContext.cpp:51:5 frame WebKit#7: 0x00000001008289a2 JavaScriptCore`ctiMasmProbeTrampoline + 338 frame WebKit#8: 0x0000466db28025be frame WebKit#9: 0x0000000100754ffc JavaScriptCore`llint_entry at LowLevelInterpreter.asm:994 frame WebKit#10: 0x0000000100738173 JavaScriptCore`vmEntryToJavaScript at LowLevelInterpreter64.asm:307 frame WebKit#11: 0x0000000101489307 JavaScriptCore`JSC::JITCode::execute(this=0x0000000106ba1520, vm=0x0000000106d00000, protoCallFrame=0x00007ffeefbfd9b8) at JITCodeInlines.h:38:38 frame WebKit#12: 0x0000000101488982 JavaScriptCore`JSC::Interpreter::executeProgram(this=0x0000000106bfd1f8, source=0x00007ffeefbff090, (null)=0x000000010d0e0000, thisObj=0x000000010d0e8020) at Interpreter.cpp:847:51 frame WebKit#13: 0x00000001017d1f9c JavaScriptCore`JSC::evaluate(globalObject=0x000000010d0e0000, source=0x00007ffeefbff090, thisValue=JSValue @ 0x00007ffeefbfef60, returnedException=0x00007ffeefbff0b0) at Completion.cpp:146:38 frame WebKit#14: 0x000000010005838f jsc`runWithOptions(globalObject=0x000000010d0e0000, options=0x00007ffeefbff620, success=0x00007ffeefbff48b) at jsc.cpp:2670:35 frame WebKit#15: 0x000000010002a0da jsc`jscmain(this=0x00007ffeefbff5a0, vm=0x0000000106d00000, globalObject=0x000000010d0e0000, success=0x00007ffeefbff48b)::$_6::operator()(JSC::VM&, GlobalObject*, bool&) const at jsc.cpp:3157:13 frame WebKit#16: 0x0000000100006eff jsc`int runJSC<jscmain(int, char**)::$_6>(options=0x00007ffeefbff620, isWorker=false, func=0x00007ffeefbff5a0)::$_6 const&) at jsc.cpp:3003:9 frame WebKit#17: 0x0000000100005988 jsc`jscmain(argc=10, argv=0x00007ffeefbff6c8) at jsc.cpp:3150:18 frame WebKit#18: 0x000000010000575e jsc`main(argc=10, argv=0x00007ffeefbff6c8) at jsc.cpp:2498:15 frame WebKit#19: 0x00007fff6cfc4da9 libdyld.dylib`start + 1 frame WebKit#20: 0x00007fff6cfc4da9 libdyld.dylib`start + 1 (lldb) The difference is that the x86_64 ctiMasmProbeTrampoline now uses the standard function prologue, and keeps %rbp pointing to trampoline function's semblance of a frame that libunwind can understand while it calls the probe function. * assembler/MacroAssemblerX86Common.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253320 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Jan 17, 2020
Object allocation sinking is missing PutHint for allocations unreachable in the graph https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. * stress/allocation-sinking-puthint-control-flow-2.js: Added. (f.handler.construct): (f): Source/JavaScriptCore: Object allocation sinking is missing PutHint for sunken allocations https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. Consider the following graph: Block #0: 1: PhantomCreateActivation() 2: PhantomNewFunction() PutHint(@2, @1, FunctionActivationPLoc) Branch(#1, #2) Block #1: 3: MaterializeCreateActivation() PutHint(@2, @3, FunctionActivationPLoc) Upsilon(@3, ^5) Jump(#3) Block #2: 4: MaterializeCreateActivation() PutHint(@2, @4, FunctionActivationPLoc) Upsilon(@4, ^5) Jump(#3) Block #3: 5: Phi() ExitOK() On Block #3, we need to emit a PutHint after the Phi, since we might exit after it. However, object allocation sinking skipped this Phi because it was checking whether the base of the location that caused us to create this Phi (@2) was live, but it's dead in the graph (there are no pointers to it). The issue is that, even though there are no pointers to the base, the location `PromotedHeapLocation(@2, FunctionActivationPLoc)` is still live, so we should PutHint to it. We fix it by checking for liveness of the location rather than its base. * dfg/DFGObjectAllocationSinkingPhase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254725 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Jan 22, 2020
Object allocation sinking is missing PutHint for allocations unreachable in the graph https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. * stress/allocation-sinking-puthint-control-flow-2.js: Added. (f.handler.construct): (f): Source/JavaScriptCore: Object allocation sinking is missing PutHint for sunken allocations https://bugs.webkit.org/show_bug.cgi?id=203799 <rdar://problem/56852162> Reviewed by Saam Barati. Consider the following graph: Block #0: 1: PhantomCreateActivation() 2: PhantomNewFunction() PutHint(@2, @1, FunctionActivationPLoc) Branch(#1, #2) Block #1: 3: MaterializeCreateActivation() PutHint(@2, @3, FunctionActivationPLoc) Upsilon(@3, ^5) Jump(#3) Block #2: 4: MaterializeCreateActivation() PutHint(@2, @4, FunctionActivationPLoc) Upsilon(@4, ^5) Jump(#3) Block #3: 5: Phi() ExitOK() On Block #3, we need to emit a PutHint after the Phi, since we might exit after it. However, object allocation sinking skipped this Phi because it was checking whether the base of the location that caused us to create this Phi (@2) was live, but it's dead in the graph (there are no pointers to it). The issue is that, even though there are no pointers to the base, the location `PromotedHeapLocation(@2, FunctionActivationPLoc)` is still live, so we should PutHint to it. We fix it by checking for liveness of the location rather than its base. * dfg/DFGObjectAllocationSinkingPhase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254866 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Feb 11, 2020
…rchability. https://bugs.webkit.org/show_bug.cgi?id=207024 Reviewed by Saam Barati. This patch applies the following changes: 1. Prefix Air and B2 dumps with a tierName prefix. The tierName prefix strings are as follows: "FTL ", "DFG ", "b3 ", "Air ", "asm " The choice to use a lowercase "b3" and "asm" with upper case "Air" is deliberate because I found this combination to be easier to read and scan as prefixes of the dump lines. See dump samples below. 2. Make DFG node IDs consistently expressed as D@<node index> e.g. D@104. The definition of the node will be the id followed by a colon e.g. D@104: This makes it easy to search references to this node anywhere in the dump. Make B3 nodes expressed as b@<node index> e.g. b@542. This also makes it searchable since there's now no ambiguity between b@542 and D@542. The choice to use a lowercase "b" and an uppercase "D" is intentional because "b@542" and "d@542" looks too similar, and I prefer to not use too much uppercase. Plus this makes the node consistent in capitalization with the tierName prefixes above of "b3 " and "DFG " respectively. Here's a sample of what the dumps now look like: DFG graph dump: <code> ... 6 55: <-- foo#DFndCW:<0x62d0000b8140, bc#65, Call, known callee: Object: 0x62d000035920 with butterfly 0x0 (Structure %AN:Function), StructureID: 12711, numArgs+this = 1, numFixup = 0, stackOffset = -16 (loc0 maps to loc16)> 3 6 55: D@79:< 3:-> ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid) 4 6 55: D@3:<!0:-> KillStack(MustGen, loc7, W:Stack(loc7), ClobbersExit, bc#71, ExitInvalid) 5 6 55: D@85:<!0:-> MovHint(Check:Untyped:D@79, MustGen, loc7, W:SideState, ClobbersExit, bc#71, ExitInvalid) 6 6 55: D@102:< 1:-> CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid) 7 6 55: D@104:<!0:-> Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:WebKit#7/w:1.000000, W:SideState, bc#74, ExitInvalid) ... </code> B3 graph dump: <code> ... b3 BB#14: ; frequency = 10.000000 b3 Predecessors: WebKit#13 b3 Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [], ExitsSideways|Reads:Top, D@79) b3 Int32 b@539 = LessThan(b@531, $100(b@578), D@102) b3 Void b@542 = Branch(b@539, Terminal, D@104) b3 Successors: Then:#2, Else:WebKit#15 ... </code> Air graph dump: <code> ... Air BB#5: ; frequency = 10.000000 Air Predecessors: #4 Air Move -96(%rbp), %rax, b@531 Air Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531 Air Branch32 LessThan, %rax, $100, b@542 Air Successors: #1, #6 ... </code> FTL disassembly dump: <code> ... Air BB#5: ; frequency = 10.000000 Air Predecessors: #4 DFG D@42:< 2:-> JSConstant(JS|PureInt, Int32, Int32: 1, bc#0, ExitInvalid) DFG D@79:< 3:-> ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid) b3 Int32 b@1 = Const32(1) b3 Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [%rax, %rbx, %rbp, %r12], ExitsSideways|Reads:Top, D@79) Air Move -96(%rbp), %rax, b@531 asm 0x4576b9c04712: mov -0x60(%rbp), %rax Air Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531 asm 0x4576b9c04716: inc %eax asm 0x4576b9c04718: jo 0x4576b9c04861 DFG D@89:< 1:-> JSConstant(JS|PureNum|UseAsOther, NonBoolInt32, Int32: 100, bc#0, ExitInvalid) DFG D@102:< 1:-> CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid) DFG D@104:<!0:-> Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:WebKit#7/w:1.000000, W:SideState, bc#74, ExitInvalid) b3 Int32 b@578 = Const32(100, D@89) b3 Int32 b@539 = LessThan(b@531, $100(b@578), D@102) b3 Void b@542 = Branch(b@539, Terminal, D@104) Air Branch32 LessThan, %rax, $100, b@542 asm 0x4576b9c0471e: cmp $0x64, %eax asm 0x4576b9c04721: jl 0x4576b9c0462f Air Successors: #1, #6 ... </code> * b3/B3BasicBlock.cpp: (JSC::B3::BasicBlock::deepDump const): * b3/B3Common.cpp: * b3/B3Common.h: * b3/B3Generate.cpp: (JSC::B3::generateToAir): * b3/B3Procedure.cpp: (JSC::B3::Procedure::dump const): * b3/B3Value.cpp: * b3/air/AirBasicBlock.cpp: (JSC::B3::Air::BasicBlock::deepDump const): (JSC::B3::Air::BasicBlock::dumpHeader const): (JSC::B3::Air::BasicBlock::dumpFooter const): * b3/air/AirCode.cpp: (JSC::B3::Air::Code::dump const): * b3/air/AirCode.h: * b3/air/AirDisassembler.cpp: (JSC::B3::Air::Disassembler::dump): * b3/air/AirGenerate.cpp: (JSC::B3::Air::prepareForGeneration): * dfg/DFGCommon.cpp: * dfg/DFGCommon.h: * dfg/DFGGraph.cpp: (JSC::DFG::Graph::dump): (JSC::DFG::Graph::dumpBlockHeader): * dfg/DFGNode.cpp: (WTF::printInternal): * ftl/FTLCompile.cpp: (JSC::FTL::compile): * ftl/FTLCompile.h: * ftl/FTLState.cpp: (JSC::FTL::State::State): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@255482 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
force-pushed
the
master
branch
3 times, most recently
from
April 30, 2020 16:55
68b7d00
to
f93530c
Compare
caiolima
pushed a commit
that referenced
this pull request
May 28, 2020
…tinuation when parent and beforeChild are siblings https://bugs.webkit.org/show_bug.cgi?id=212116 <rdar://problem/62993844> Reviewed by Simon Fraser. Source/WebCore: This patch fixes the case when a nested fragmented context has a spanner and we try to form a continuation while this nested fragmented context is being destroyed. 1. The continuation is triggered by a style change that turns a previously out-of-flow block container into an inflow box (and the parent inline level container can't have the box as a direct child anymore). 2. An unrelated style change nukes the nested fragmented context. We need to "re-assign" the spanner to the parent fragment. These 2 changes are split into 2 phases; first we take care of the tree mutation triggered by the continuation (updateRendererStyle), while we do the fragmented context cleanup (updateAfterDescendants) in a separate step. This 2 phase setup confuses the "where to put this spanner" logic. This patch addresses the issue by keeping the spanner inside the about-to-be-destroyed fragmented context while forming the continuation (phase #1) and let the second phase (updateAfterDescendants) deal with the spanner moving. Test: fast/multicol/nested-multicol-with-spanner-and-continuation.html * rendering/updating/RenderTreeBuilderMultiColumn.cpp: (WebCore::isValidColumnSpanner): LayoutTests: * fast/multicol/nested-multicol-with-spanner-and-continuation-expected.txt: Added. * fast/multicol/nested-multicol-with-spanner-and-continuation.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@262093 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Jul 6, 2020
https://bugs.webkit.org/show_bug.cgi?id=213436 Reviewed by Antti Koivisto. Source/WebCore: 1. The table generates a principal block container box called the table wrapper box that contains the table box itself and any caption boxes. 2. The table wrapper box establishes a block formatting context, and the table box establishes a table formatting context. 3. The computed values of properties 'position', 'float', 'margin-*', 'top', 'right', 'bottom', and 'left' on the table element are used on the table wrapper box and not the table box; all other values of non-inheritable properties are used on the table box and not the table wrapper box. 4. In a block formatting context, each box's left outer edge touches the left edge of the containing block. This is true even in the presence of floats, unless the box establishes a new block formatting context (in which case the box itself may become narrower due to the floats) Now consider the following case: <div style="display: block; width: 500px;"> <div style="float: left; width: 100px;"></div> <div style="display: table; width: 10%;"></div> </div> 1. We create a table wrapper box to wrap the "display: table" block level box (#1). 2. The table wrapper box's width property is set to auto (#3). 3. Since it establishes a new block formatting context, the available horizontal space gets shrunk by the float (#4) 4. The table wrapper box's used width computes to 500px - 100px -> 400px; Now we are inside the BFC established by the table wrapper box and try to resolve the table's width -> %10. According to the normal BFC rules, it should compute to 10% of the containing block's logical width: 400px -> 40px. However in practice it computes to 50px (10% of 500px). Similar setup with non-table content would resolve the inner block level box's width to 40px; <div style="display: block; width: 500px"> <div style="float: left; width: 100px;"></div> <div style="display: block; overflow: hidden;"> <div style="display: block; width: 10%"></div> </div> </div> This needs clarification. Test: fast/layoutformattingcontext/float-avoider-available-horizontal-space3.html * layout/FormattingContext.h: (WebCore::Layout::FormattingContext::isTableWrapperBlockFormattingContext const): * layout/blockformatting/BlockFormattingContext.cpp: (WebCore::Layout::BlockFormattingContext::layoutInFlowContent): * layout/blockformatting/tablewrapper/TableWrapperBlockFormattingContext.cpp: (WebCore::Layout::TableWrapperBlockFormattingContext::computeWidthAndMarginForTableBox): * layout/blockformatting/tablewrapper/TableWrapperBlockFormattingContext.h: LayoutTests: * fast/layoutformattingcontext/float-avoider-available-horizontal-space3-expected.html: Added. * fast/layoutformattingcontext/float-avoider-available-horizontal-space3.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@263327 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima
pushed a commit
that referenced
this pull request
Sep 24, 2020
https://bugs.webkit.org/show_bug.cgi?id=202582 Reviewed by Yusuke Suzuki and Keith Miller. JSTests: Provided microbenchmarks test receivers that are half-sorted: 50% of their items and item pairs (to accomodate merge sort) are at the right place. Arrays of multiple sizes (8/24/64 items) are tested with both userland and default comparator (to cover bucket sort). * ChakraCore/test/Array/array_sort.baseline-jsc: Fix typo in error message. * microbenchmarks/array-prototype-sort-large-array-comparator.js: Added. * microbenchmarks/array-prototype-sort-large-array.js: Added. * microbenchmarks/array-prototype-sort-medium-array-comparator.js: Added. * microbenchmarks/array-prototype-sort-medium-array.js: Added. * microbenchmarks/array-prototype-sort-small-array-comparator.js: Added. * microbenchmarks/array-prototype-sort-small-array.js: Added. * mozilla/js1_5/Array/regress-157652.js: Skip sorting sparse array of UINT_MAX size. * stress/regress-188577.js: Replace sort() with unshift() and refactor. Source/JavaScriptCore: This patch implements the spec change [1] that reduces amount of cases resulting in an implementation-defined sort order, aligning JSC with V8 and SpiderMonkey. To achieve this, we collect all existing non-undefined receiver elements to a temporary array, sort it, and write back sorted items, followed by `undefined` values and holes. This change is proven to be web-compatible (shipping since Chrome 76) and neutral on peak memory consumption in the wild. Although we can unobservably detect sparse receivers, we can't avoid creating a temporary array for common case since userland comparators may throw; string sorting won't measurably benefit from this, only increasing code complexity. This change uses @putByValDirect unless the spec requires [[Set]], avoids using closure variables, and adds a few drive-by optimizations, resulting in ~22% faster string sorting and 13% speed-up for userland comparators. Dromaeo/jslib is neutral. [1]: tc39/ecma262#1585 * builtins/ArrayPrototype.js: (sort.stringComparator): Optimization #1: replace char-by-char comparison loop with > operator, aligning JSC with V8 and SpiderMonkey. This semantically equivalent change alone is a ~15% progression for string sort. (sort.compact): (sort.commit): Optimization #2: copy large non-numeric arrays in a loop rather than @appendMemcpy. Using the latter unconditionally regresses provided microbenchmarks. (sort.merge): Optimization #3: replace `typeof` check and negation with strict equality. (sort.mergeSort): Optimization #4: always return sorted array instead of copying, even if it's the buffer. Tweak: create the buffer with correct length. (sort.bucketSort): Optimization #5: avoid emitting 2 extra get_by_val ops by saving bucket lookup to a variable. Tweak: create new bucket via array literal. (sort): Fix typo in error message. (sort.compactSparse): Deleted. (sort.compactSlow): Deleted. (sort.comparatorSort): Deleted. (sort.stringSort): Deleted. * runtime/ObjectConstructor.cpp: (JSC::ObjectConstructor::finishCreation): Remove @object.@getPrototypeOf as it's now unused and we have @getPrototypeOf intrinsic anyway. LayoutTests: While adding new LayoutTests for JS-only features is undesirable, it's a quick-and-dirty way to import the tests [1] and fix the call count/order of observable operations via debug() and text expectations. The tests are imported into LayoutTests/js/dom instead of LayoutTests/js for run-javascriptcore-tests to ignore them as they require array-sort-harness.js. These files will be removed shortly in favor of thorough test262 coverage, which is required for the proposal [2] to be merged. [1]: https://gist.github.com/szuend/05ae15b4e1329b264ab4c9a1cda09242 [2]: tc39/ecma262#1585 * TestExpectations: Mark a test as slow. * js/dom/array-sort-*-expected.txt: Added. * js/dom/array-sort-*.html: Added. * js/dom/script-tests/array-sort-*.js: Added. * js/resources/array-sort-harness.js: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@267514 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just to see pretty diff