[JSC] Shrink some of Vectors in JSC
Created attachment 425110 [details] Patch
Created attachment 425111 [details] Patch
Created attachment 425113 [details] Patch
Comment on attachment 425113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425113&action=review > Source/JavaScriptCore/bytecode/DeleteByStatus.cpp:137 > + result.shrinkToFit(); For cases like this, might it make sense to have > Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:129 > m_routines.shrink(dstIndex); > + m_routines.shrinkToFit(); Could these be combined into to just m_routines.shrinkCapacity(dstIndex)?
Created attachment 425115 [details] Patch
Comment on attachment 425113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425113&action=review >> Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:129 >> + m_routines.shrinkToFit(); > > Could these be combined into to just m_routines.shrinkCapacity(dstIndex)? Sounds good. Changed.
Created attachment 425117 [details] Patch
Created attachment 425119 [details] Patch
Created attachment 425120 [details] Patch
Created attachment 425121 [details] Patch
Comment on attachment 425121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425121&action=review > Source/JavaScriptCore/bytecode/GetterSetterAccessCase.cpp:44 > +GetterSetterAccessCase::GetterSetterAccessCase(VM& vm, JSCell* owner, AccessType accessType, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, bool viaProxy, WatchpointSet* additionalSet, JSObject* customSlotBase, RefPtr<PolyProtoAccessChain> prototypeAccessChain) Should that be a RefPtr<>&&? > Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h:59 > + JSObject* customSlotBase, Optional<DOMAttributeAnnotation>, RefPtr<PolyProtoAccessChain>); RefPtr<>&&? > Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h:62 > + const ObjectPropertyConditionSet&, RefPtr<PolyProtoAccessChain>, bool viaProxy = false, Ditto > Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h:73 > + GetterSetterAccessCase(VM&, JSCell*, AccessType, CacheableIdentifier, PropertyOffset, Structure*, const ObjectPropertyConditionSet&, bool viaProxy, WatchpointSet* additionalSet, JSObject* customSlotBase, RefPtr<PolyProtoAccessChain>); Ditto > Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.cpp:36 > +IntrinsicGetterAccessCase::IntrinsicGetterAccessCase(VM& vm, JSCell* owner, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain> prototypeAccessChain) Ditto > Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.cpp:42 > +std::unique_ptr<AccessCase> IntrinsicGetterAccessCase::create(VM& vm, JSCell* owner, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain> prototypeAccessChain) Ditto > Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.h:45 > + static std::unique_ptr<AccessCase> create(VM&, JSCell*, CacheableIdentifier, PropertyOffset, Structure*, const ObjectPropertyConditionSet&, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain>); Ditto > Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.h:52 > + IntrinsicGetterAccessCase(VM&, JSCell*, CacheableIdentifier, PropertyOffset, Structure*, const ObjectPropertyConditionSet&, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain>); Ditto > Source/JavaScriptCore/bytecode/ProxyableAccessCase.cpp:41 > +std::unique_ptr<AccessCase> ProxyableAccessCase::create(VM& vm, JSCell* owner, AccessType type, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, bool viaProxy, WatchpointSet* additionalSet, RefPtr<PolyProtoAccessChain> prototypeAccessChain) Ditto
Comment on attachment 425121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425121&action=review Thanks! >> Source/JavaScriptCore/bytecode/GetterSetterAccessCase.cpp:44 >> +GetterSetterAccessCase::GetterSetterAccessCase(VM& vm, JSCell* owner, AccessType accessType, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, bool viaProxy, WatchpointSet* additionalSet, JSObject* customSlotBase, RefPtr<PolyProtoAccessChain> prototypeAccessChain) > > Should that be a RefPtr<>&&? Fixed. >> Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h:59 >> + JSObject* customSlotBase, Optional<DOMAttributeAnnotation>, RefPtr<PolyProtoAccessChain>); > > RefPtr<>&&? Fixed. >> Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h:62 >> + const ObjectPropertyConditionSet&, RefPtr<PolyProtoAccessChain>, bool viaProxy = false, > > Ditto Fixed. >> Source/JavaScriptCore/bytecode/GetterSetterAccessCase.h:73 >> + GetterSetterAccessCase(VM&, JSCell*, AccessType, CacheableIdentifier, PropertyOffset, Structure*, const ObjectPropertyConditionSet&, bool viaProxy, WatchpointSet* additionalSet, JSObject* customSlotBase, RefPtr<PolyProtoAccessChain>); > > Ditto Fixed. >> Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.cpp:36 >> +IntrinsicGetterAccessCase::IntrinsicGetterAccessCase(VM& vm, JSCell* owner, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain> prototypeAccessChain) > > Ditto Fixed. >> Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.cpp:42 >> +std::unique_ptr<AccessCase> IntrinsicGetterAccessCase::create(VM& vm, JSCell* owner, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain> prototypeAccessChain) > > Ditto Fixed. >> Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.h:45 >> + static std::unique_ptr<AccessCase> create(VM&, JSCell*, CacheableIdentifier, PropertyOffset, Structure*, const ObjectPropertyConditionSet&, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain>); > > Ditto Fixed. >> Source/JavaScriptCore/bytecode/IntrinsicGetterAccessCase.h:52 >> + IntrinsicGetterAccessCase(VM&, JSCell*, CacheableIdentifier, PropertyOffset, Structure*, const ObjectPropertyConditionSet&, JSFunction* intrinsicFunction, RefPtr<PolyProtoAccessChain>); > > Ditto Fixed. >> Source/JavaScriptCore/bytecode/ProxyableAccessCase.cpp:41 >> +std::unique_ptr<AccessCase> ProxyableAccessCase::create(VM& vm, JSCell* owner, AccessType type, CacheableIdentifier identifier, PropertyOffset offset, Structure* structure, const ObjectPropertyConditionSet& conditionSet, bool viaProxy, WatchpointSet* additionalSet, RefPtr<PolyProtoAccessChain> prototypeAccessChain) > > Ditto Fixed.
Created attachment 425122 [details] Patch
Created attachment 425124 [details] Patch
Comment on attachment 425121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425121&action=review > Source/JavaScriptCore/bytecode/PolyProtoAccessChain.cpp:80 > + return adoptRef(*new PolyProtoAccessChain(WTFMove(chain))); It's unfortunate that we are implementing a tryCreate() function here but we're going to crash if new fails. Can we use a placement new using tryMalloc so that we can fail without crashing?
(In reply to Mark Lam from comment #15) > Comment on attachment 425121 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=425121&action=review > > > Source/JavaScriptCore/bytecode/PolyProtoAccessChain.cpp:80 > > + return adoptRef(*new PolyProtoAccessChain(WTFMove(chain))); > > It's unfortunate that we are implementing a tryCreate() function here but > we're going to crash if new fails. Can we use a placement new using > tryMalloc so that we can fail without crashing? Make that tryFastMalloc().
Comment on attachment 425121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425121&action=review >>> Source/JavaScriptCore/bytecode/PolyProtoAccessChain.cpp:80 >>> + return adoptRef(*new PolyProtoAccessChain(WTFMove(chain))); >> >> It's unfortunate that we are implementing a tryCreate() function here but we're going to crash if new fails. Can we use a placement new using tryMalloc so that we can fail without crashing? > > Make that tryFastMalloc(). This tryCreate has meaning: it fails when the passed argument meets particular condition. Returning nullptr when OOM will break our IC's semantics.
Comment on attachment 425121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425121&action=review >>>> Source/JavaScriptCore/bytecode/PolyProtoAccessChain.cpp:80 >>>> + return adoptRef(*new PolyProtoAccessChain(WTFMove(chain))); >>> >>> It's unfortunate that we are implementing a tryCreate() function here but we're going to crash if new fails. Can we use a placement new using tryMalloc so that we can fail without crashing? >> >> Make that tryFastMalloc(). > > This tryCreate has meaning: it fails when the passed argument meets particular condition. > Returning nullptr when OOM will break our IC's semantics. ok, thanks for clarifying.
Committed r275490 (236147@main): <https://commits.webkit.org/236147@main>
<rdar://problem/76251180>