RESOLVED FIXED 171474
[JSC] LabelScopePtr is not necessary
https://bugs.webkit.org/show_bug.cgi?id=171474
Summary [JSC] LabelScopePtr is not necessary
Yusuke Suzuki
Reported 2017-04-29 01:49:50 PDT
At the time of introducing LabelScopePtr, there is code that copying m_labelScope. So, m_labelScope is Vector<> instead of SegmentedVector and a bit special handle (LabelScopePtr) is needed. But right now, there is no code copying m_labelScopes. So, we can take the same design to Label, RegisterID etc. Use SegmentedVector and usual Ref<> / RefPtr<>. Then drops LabelScopePtr. And we also makes LabelScope noncopyable.
Attachments
Patch (21.78 KB, patch)
2017-04-29 03:58 PDT, Yusuke Suzuki
ggaren: review+
Yusuke Suzuki
Comment 1 2017-04-29 03:58:05 PDT
Yusuke Suzuki
Comment 2 2017-04-29 03:59:01 PDT
Comment on attachment 308664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308664&action=review > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:72 > + // loop condition is a workaround. Old code says so. But I'm not sure the above claim is still valid.
Geoffrey Garen
Comment 3 2017-04-29 09:18:55 PDT
Comment on attachment 308664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308664&action=review r=me > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:66 > +static inline void reclaim(T& segmentedVector) I would call this "shrinkToFit" like Vector<T>::shrinkToFit. >> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:72 >> + // loop condition is a workaround. > > Old code says so. But I'm not sure the above claim is still valid. I think it's good to remove workarounds like this over time. Maybe we should remove this.
Yusuke Suzuki
Comment 4 2017-04-29 09:30:21 PDT
Comment on attachment 308664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308664&action=review Thanks! >> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:66 >> +static inline void reclaim(T& segmentedVector) > > I would call this "shrinkToFit" like Vector<T>::shrinkToFit. Fixed. >>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:72 >>> + // loop condition is a workaround. >> >> Old code says so. But I'm not sure the above claim is still valid. > > I think it's good to remove workarounds like this over time. Maybe we should remove this. Dropped. Just use cleaner way.
Yusuke Suzuki
Comment 5 2017-04-29 09:33:08 PDT
Note You need to log in before you can comment on or make changes to this bug.