Reduce binary size by purging C++ type information in Objective-C fields and parameters
Created attachment 387068 [details] Patch
Created attachment 387070 [details] Patch
<rdar://problem/58398934>
Created attachment 387071 [details] Patch
Comment on attachment 387071 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387071&action=review > Source/WTF/ChangeLog:19 > + This patch introduces NativeRef<T>. This is similar to Ref while it does not have any ownership. So it Native => Naked.
Comment on attachment 387071 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387071&action=review Nice!! r=me > Source/WTF/ChangeLog:8 > + Objective-C has reflection mechanism. This means that fields, methods, and their types "has reflection" => "has a reflection" > Source/WTF/ChangeLog:24 > + 2. T* / T& is in Objective-C fields or paramter types (including a return type). paramter => parameter
Comment on attachment 387071 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387071&action=review >> Source/WTF/ChangeLog:8 >> + Objective-C has reflection mechanism. This means that fields, methods, and their types > > "has reflection" => "has a reflection" actually, probably this should just be: "mechanism" => "mechanisms" since ObjC has more than one kind of reflection
Created attachment 387073 [details] Patch iOS build fix
Comment on attachment 387073 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387073&action=review > Source/WTF/ChangeLog:36 > + becomes available in our platforms, we can consider it. But using NakedRef / NakedPtr is not harmless. Do you mean "is harmless" instead of "is not harmless", as in there is effectively no cost to using NakedRef / NakedPtr?
I'm now looking WebKitLegacy crash.
(In reply to Yusuke Suzuki from comment #10) > I'm now looking WebKitLegacy crash. OK, found the thing. Signature in WebViewInternal.h and WebView.mm were different. I really think this must be compile error even in Objective-C!
(In reply to Yusuke Suzuki from comment #11) > (In reply to Yusuke Suzuki from comment #10) > > I'm now looking WebKitLegacy crash. > > OK, found the thing. Signature in WebViewInternal.h and WebView.mm were > different. > I really think this must be compile error even in Objective-C! I'll double-check and upload the patch for EWS before landing.
Created attachment 387081 [details] Patch WebKitLegacy fix
How do we ensure that people correctly use Naked<> in future? It's going to be very easy for new code to regress this again, right?
(In reply to Simon Fraser (smfr) from comment #14) > How do we ensure that people correctly use Naked<> in future? It's going to > be very easy for new code to regress this again, right? I think we should track binary size as we are doing in performance testing.
Created attachment 387147 [details] Patch Fix
I've ensured that iOS simulator API tests pass locally.
Committed r254241: <https://trac.webkit.org/changeset/254241>
(In reply to Yusuke Suzuki from comment #15) > (In reply to Simon Fraser (smfr) from comment #14) > > How do we ensure that people correctly use Naked<> in future? It's going to > > be very easy for new code to regress this again, right? > > I think we should track binary size as we are doing in performance testing. Is this something we could automate at build time? e.g. could we add a script phase (similar to check-for-weak-vtables-and-externals) that checks for this specific issue, C++ type information in Objective-C fields and parameters?
(In reply to Sam Weinig from comment #19) > (In reply to Yusuke Suzuki from comment #15) > > (In reply to Simon Fraser (smfr) from comment #14) > > > How do we ensure that people correctly use Naked<> in future? It's going to > > > be very easy for new code to regress this again, right? > > > > I think we should track binary size as we are doing in performance testing. > > Is this something we could automate at build time? e.g. could we add a > script phase (similar to check-for-weak-vtables-and-externals) that checks > for this specific issue, C++ type information in Objective-C fields and > parameters? PoC of this exists here :) https://bugs.webkit.org/show_bug.cgi?id=205968 Discussed with Simon and we can put this type of script as the same way to Scripts/check-for-exit-time-destructors etc.
(In reply to Yusuke Suzuki from comment #20) > (In reply to Sam Weinig from comment #19) > > (In reply to Yusuke Suzuki from comment #15) > > > (In reply to Simon Fraser (smfr) from comment #14) > > > > How do we ensure that people correctly use Naked<> in future? It's going to > > > > be very easy for new code to regress this again, right? > > > > > > I think we should track binary size as we are doing in performance testing. > > > > Is this something we could automate at build time? e.g. could we add a > > script phase (similar to check-for-weak-vtables-and-externals) that checks > > for this specific issue, C++ type information in Objective-C fields and > > parameters? > > PoC of this exists here :) https://bugs.webkit.org/show_bug.cgi?id=205968 > Discussed with Simon and we can put this type of script as the same way to > Scripts/check-for-exit-time-destructors etc. Very nice.