[JSC] Make more DFG/FTL data FixedVector/Vector
Created attachment 426321 [details] Patch
Created attachment 426322 [details] Patch
Comment on attachment 426322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426322&action=review > Source/JavaScriptCore/bytecode/Operands.h:141 > + template<typename U, typename V> friend class Operands; No need for the "U" and "V" here. > Source/JavaScriptCore/dfg/DFGJITCompiler.h:269 > + unsigned appendOSRExit(OSRExit&& exit) This creates a 2^32 maximum. Does something check that and prevent overflow? > Source/JavaScriptCore/dfg/DFGJITCompiler.h:276 > + unsigned appendSpeculationRecovery(const SpeculationRecovery& recovery) Ditto. > Source/JavaScriptCore/dfg/DFGJITCompiler.h:394 > +public: > + Vector<DFG::OSREntryData> m_osrEntry; > + Vector<DFG::OSRExit> m_osrExit; > + Vector<DFG::SpeculationRecovery> m_speculationRecovery; > +private: This is inelegant. Is there a way to do this without making the vectors public? Even accessor functions that return references would be more elegant than this. But also could use friend maybe? > Source/JavaScriptCore/ftl/FTLOSRExit.cpp:88 > + unsigned index = state.jitCode->m_osrExit.size(); Same 2^32 question.
Comment on attachment 426322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426322&action=review >> Source/JavaScriptCore/dfg/DFGJITCompiler.h:269 >> + unsigned appendOSRExit(OSRExit&& exit) > > This creates a 2^32 maximum. Does something check that and prevent overflow? Vector crashes on overflow. And there is no way to achieve that since DFG / FTL has node amount limit (10000) >> Source/JavaScriptCore/dfg/DFGJITCompiler.h:276 >> + unsigned appendSpeculationRecovery(const SpeculationRecovery& recovery) > > Ditto. Ditto. >> Source/JavaScriptCore/dfg/DFGJITCompiler.h:394 >> +private: > > This is inelegant. Is there a way to do this without making the vectors public? Even accessor functions that return references would be more elegant than this. But also could use friend maybe? I don't think accessor function is better since we WTFMove(them). I don't think we expect that when we are using accessor functions. friend class would be better. >> Source/JavaScriptCore/ftl/FTLOSRExit.cpp:88 >> + unsigned index = state.jitCode->m_osrExit.size(); > > Same 2^32 question. Ditto.
Committed r276224 (236706@main): <https://commits.webkit.org/236706@main>
Comment on attachment 426322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426322&action=review >>> Source/JavaScriptCore/dfg/DFGJITCompiler.h:394 >>> +private: >> >> This is inelegant. Is there a way to do this without making the vectors public? Even accessor functions that return references would be more elegant than this. But also could use friend maybe? > > I don't think accessor function is better since we WTFMove(them). I don't think we expect that when we are using accessor functions. > friend class would be better. If what we are doing is moving vectors out, then the style of access function I would recommend is “take“ which includes the move (or std::exchange) and returns the moved object. This does create better encapsulation than just having public data members.
rdar://76809579