Re: [webkit-dev] Accidental binary bloating via C/C++ class/struct + Objective-C

[Yusuke Suzuki]

> On Jan 13, 2020, at 19:28, Darin Adler  wrote:
> 
>> On Jan 13, 2020, at 5:52 PM, Yusuke Suzuki > > wrote:
>> 
>> We can purge type-encoding-string if we use Objective-C NS_DIRECT feature 
>> (which makes Objective-C function as C function calling convention, removing 
>> metadata).
>> However, this does not work universally: with NS_DIRECT, Objective-C 
>> override does not work. This means we need to be extra-careful when using it.
> 
> Yes, we need to be careful, but NS_DIRECT is likely to have more benefit than 
> just binary shrinking, and it should do even more binary shrinking than 
> hiding types from Objective-C. Use of NS_DIRECT will likely help performance, 
> sometimes in a measurable way.

Right. I guess that many parts of code requiring high-performance are already 
in C++ in WebKit project.
But still, we should explore the places we can use NS_DIRECT, e.g. internal 
accessors of Objective-C classes. We could get benefit.
One thing I would like to check is JSC APIs, since typical operation inside JSC 
APIs are very small.
If one API is calling another internal objective-C methods, which are never 
overridden, we could get benefit by annotating these internal methods w/ 
NS_DIRECT.

> 
> However, besides the risk of making something “non-overridable”, I think it’s 
> only available in new versions of the clang compiler.
> 
>> So, as a simple, but effective work-around, in the above patch, we 
>> introduced NakedRef / NakedPtr. This is basically raw pointer / raw 
>> reference to T, with a wrapper class.
>> This leverages the behavior of Objective-C compiler’s mechanism “one-level 
>> deep type information collection”. Since NakedRef / NakedPtr 
>> introduces one-level deep field,
>> Objective-C compiler does not collect the type information of T if 
>> NakedPtr is included in the fields / signatures, while the compiler 
>> collects information when T* is used.
> 
> Very exciting. Does this cover all the cases we care about? Does come up for 
> types that are not references or pointers? Maybe we can pass arguments by 
> const reference? What about return values?

Many cases are covered. Non-covered part is code passing C/C++ struct/class as 
values.
We could shrink it if we allocate it by using std::unique_ptr / RefPtr / Ref 
and passing std::unique_ptr&& / RefPtr&& / Ref&& (since they are also 
introducing one-level nested structure).
For the places using raw pointers / raw references, we can attempt to use 
NakedPtr<> / NakedRef<>.

> 
>> ## Future work
>> 
>> We would like to avoid including such types accidentally in Objective-C. We 
>> should introduce build-time hook script which detects such a thing.
>> I uploaded the PoC script in https://bugs.webkit.org/show_bug.cgi?id=205968 
>> , and I’m personally 
>> planning to introduce such a hook into a part of build process.
> 
> Beautiful. Well worth doing.

Yeah, we should do it. While the above two patches removed too large 
type-encoding-strings (like, one type-encoding-string took more than 100KB…),
we still have many of very long type-encoding-strings (like 6KB). Removing this 
can reduce binary-size. And it could be nice for performance if this string is 
used some part of Objective-C runtime hash-table etc. (Not sure whether it 
happens) by avoiding including such a large string into a hash table.

-Yusuke

> 
> Thanks!
> 
> — Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Accidental binary bloating via C/C++ class/struct + Objective-C

[Darin Adler]

> On Jan 13, 2020, at 5:52 PM, Yusuke Suzuki  wrote:
> 
> We can purge type-encoding-string if we use Objective-C NS_DIRECT feature 
> (which makes Objective-C function as C function calling convention, removing 
> metadata).
> However, this does not work universally: with NS_DIRECT, Objective-C override 
> does not work. This means we need to be extra-careful when using it.

Yes, we need to be careful, but NS_DIRECT is likely to have more benefit than 
just binary shrinking, and it should do even more binary shrinking than hiding 
types from Objective-C. Use of NS_DIRECT will likely help performance, 
sometimes in a measurable way.

However, besides the risk of making something “non-overridable”, I think it’s 
only available in new versions of the clang compiler.

> So, as a simple, but effective work-around, in the above patch, we introduced 
> NakedRef / NakedPtr. This is basically raw pointer / raw reference to 
> T, with a wrapper class.
> This leverages the behavior of Objective-C compiler’s mechanism “one-level 
> deep type information collection”. Since NakedRef / NakedPtr introduces 
> one-level deep field,
> Objective-C compiler does not collect the type information of T if 
> NakedPtr is included in the fields / signatures, while the compiler 
> collects information when T* is used.

Very exciting. Does this cover all the cases we care about? Does come up for 
types that are not references or pointers? Maybe we can pass arguments by const 
reference? What about return values?

> ## Future work
> 
> We would like to avoid including such types accidentally in Objective-C. We 
> should introduce build-time hook script which detects such a thing.
> I uploaded the PoC script in https://bugs.webkit.org/show_bug.cgi?id=205968 
> , and I’m personally planning 
> to introduce such a hook into a part of build process.

Beautiful. Well worth doing.

Thanks!

— Darin___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Accidental binary bloating via C/C++ class/struct + Objective-C

[Joseph Pecoraro]

This is a great idea!

- Joe

> On Jan 13, 2020, at 5:52 PM, Yusuke Suzuki  wrote:
> 
> Hello WebKittens,
> 
> I recently striped 830KB binary size in WebKit just by using a work-around.
> This email describes what happened so far, to prevent from happening again.
> 
> ## Problem
> 
> When C/C++ struct/class is included in field types and method types in 
> Objective-C, Objective-C compiler puts type-enconding-string which gathers 
> type information one-leve deep for C/C++ struct/class if
> 
> 1. The type is a pointer to C/C++ struct/class
> 2. The type is a value of C/C++ struct/class
> 3. The type is a reference to C/C++ struct/class
> 
> However, our WebKit C/C++ struct/class is typically very complex type using a 
> lot of templates. Unfortunately, Objective-C compiler includes expanded 
> template definition as a string and adds it as a type-enconding-string into 
> the release binary!
> 
> For example, https://trac.webkit.org/changeset/254152/webkit 
>  is removing JSC::VM& from 
> Objective-C signature, and it reduces 200KB binary size!
> Another example is https://trac.webkit.org/changeset/254241/webkit 
> , which removes a lot of 
> WebCore::WebView* etc. from Objective-C method signature, and reduces 630KB 
> binary.
> 
> ## Solution for now
> 
> We can purge type-encoding-string if we use Objective-C NS_DIRECT feature 
> (which makes Objective-C function as C function calling convention, removing 
> metadata).
> However, this does not work universally: with NS_DIRECT, Objective-C override 
> does not work. This means we need to be extra-careful when using it.
> 
> So, as a simple, but effective work-around, in the above patch, we introduced 
> NakedRef / NakedPtr. This is basically raw pointer / raw reference to 
> T, with a wrapper class.
> This leverages the behavior of Objective-C compiler’s mechanism “one-level 
> deep type information collection”. Since NakedRef / NakedPtr introduces 
> one-level deep field,
> Objective-C compiler does not collect the type information of T if 
> NakedPtr is included in the fields / signatures, while the compiler 
> collects information when T* is used.
> 
> So, if you are using T& / T* C/C++ struct/class in Objective-C, let’s convert 
> it to NakedRef / NakedPtr. Then you could save much binary size 
> immediately without causing any performance problem.
> 
> ## Future work
> 
> We would like to avoid including such types accidentally in Objective-C. We 
> should introduce build-time hook script which detects such a thing.
> I uploaded the PoC script in https://bugs.webkit.org/show_bug.cgi?id=205968 
> , and I’m personally planning 
> to introduce such a hook into a part of build process.
> 
> 
> -Yusuke
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Accidental binary bloating via C/C++ class/struct + Objective-C

[Filip Pizlo]

Wow, that sounds like an awesome find!

-Filip

> On Jan 13, 2020, at 5:53 PM, Yusuke Suzuki  wrote:
> 
> Hello WebKittens,
> 
> I recently striped 830KB binary size in WebKit just by using a work-around.
> This email describes what happened so far, to prevent from happening again.
> 
> ## Problem
> 
> When C/C++ struct/class is included in field types and method types in 
> Objective-C, Objective-C compiler puts type-enconding-string which gathers 
> type information one-leve deep for C/C++ struct/class if
> 
> 1. The type is a pointer to C/C++ struct/class
> 2. The type is a value of C/C++ struct/class
> 3. The type is a reference to C/C++ struct/class
> 
> However, our WebKit C/C++ struct/class is typically very complex type using a 
> lot of templates. Unfortunately, Objective-C compiler includes expanded 
> template definition as a string and adds it as a type-enconding-string into 
> the release binary!
> 
> For example, https://trac.webkit.org/changeset/254152/webkit is removing 
> JSC::VM& from Objective-C signature, and it reduces 200KB binary size!
> Another example is https://trac.webkit.org/changeset/254241/webkit, which 
> removes a lot of WebCore::WebView* etc. from Objective-C method signature, 
> and reduces 630KB binary.
> 
> ## Solution for now
> 
> We can purge type-encoding-string if we use Objective-C NS_DIRECT feature 
> (which makes Objective-C function as C function calling convention, removing 
> metadata).
> However, this does not work universally: with NS_DIRECT, Objective-C override 
> does not work. This means we need to be extra-careful when using it.
> 
> So, as a simple, but effective work-around, in the above patch, we introduced 
> NakedRef / NakedPtr. This is basically raw pointer / raw reference to 
> T, with a wrapper class.
> This leverages the behavior of Objective-C compiler’s mechanism “one-level 
> deep type information collection”. Since NakedRef / NakedPtr introduces 
> one-level deep field,
> Objective-C compiler does not collect the type information of T if 
> NakedPtr is included in the fields / signatures, while the compiler 
> collects information when T* is used.
> 
> So, if you are using T& / T* C/C++ struct/class in Objective-C, let’s convert 
> it to NakedRef / NakedPtr. Then you could save much binary size 
> immediately without causing any performance problem.
> 
> ## Future work
> 
> We would like to avoid including such types accidentally in Objective-C. We 
> should introduce build-time hook script which detects such a thing.
> I uploaded the PoC script in https://bugs.webkit.org/show_bug.cgi?id=205968, 
> and I’m personally planning to introduce such a hook into a part of build 
> process.
> 
> 
> -Yusuke
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Accidental binary bloating via C/C++ class/struct + Objective-C

[Yusuke Suzuki]

Hello WebKittens,

I recently striped 830KB binary size in WebKit just by using a work-around.
This email describes what happened so far, to prevent from happening again.

## Problem

When C/C++ struct/class is included in field types and method types in 
Objective-C, Objective-C compiler puts type-enconding-string which gathers type 
information one-leve deep for C/C++ struct/class if

1. The type is a pointer to C/C++ struct/class
2. The type is a value of C/C++ struct/class
3. The type is a reference to C/C++ struct/class

However, our WebKit C/C++ struct/class is typically very complex type using a 
lot of templates. Unfortunately, Objective-C compiler includes expanded 
template definition as a string and adds it as a type-enconding-string into the 
release binary!

For example, https://trac.webkit.org/changeset/254152/webkit 
 is removing JSC::VM& from 
Objective-C signature, and it reduces 200KB binary size!
Another example is https://trac.webkit.org/changeset/254241/webkit 
, which removes a lot of 
WebCore::WebView* etc. from Objective-C method signature, and reduces 630KB 
binary.

## Solution for now

We can purge type-encoding-string if we use Objective-C NS_DIRECT feature 
(which makes Objective-C function as C function calling convention, removing 
metadata).
However, this does not work universally: with NS_DIRECT, Objective-C override 
does not work. This means we need to be extra-careful when using it.

So, as a simple, but effective work-around, in the above patch, we introduced 
NakedRef / NakedPtr. This is basically raw pointer / raw reference to T, 
with a wrapper class.
This leverages the behavior of Objective-C compiler’s mechanism “one-level deep 
type information collection”. Since NakedRef / NakedPtr introduces 
one-level deep field,
Objective-C compiler does not collect the type information of T if NakedPtr 
is included in the fields / signatures, while the compiler collects information 
when T* is used.

So, if you are using T& / T* C/C++ struct/class in Objective-C, let’s convert 
it to NakedRef / NakedPtr. Then you could save much binary size 
immediately without causing any performance problem.

## Future work

We would like to avoid including such types accidentally in Objective-C. We 
should introduce build-time hook script which detects such a thing.
I uploaded the PoC script in https://bugs.webkit.org/show_bug.cgi?id=205968 
, and I’m personally planning 
to introduce such a hook into a part of build process.


-Yusuke___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Use of [[maybe_unused]]

[Alex Christensen]

> On Jan 13, 2020, at 4:08 PM, Suzuki, Basuke  wrote:
> 
> `sessionID` is used in RELEASE_LOG_IF_ALLOWED() and we have empty 
> implementation of RELEASE_LOG() so that it's ended up with unused parameter 
> warning of sessionID. We can add UNUSED_PARAM(sessionID) in this case, but 
> [[maybe_unused]] is more correct choice to describe the code because 
> sessionID is actually used.
In this case you could use RELEASE_LOG_DISABLED but I agree [[maybe_unused]] 
would be better.  I’m ok with it as long as all the ports are OK with it, 
including support in their oldest supported compiler.

> This member variable is only used in COCOA port. 
> (https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp#L203)
> 
> We can add UNUSED_PARAM(isHTTPSUpgradeEnabled) in our platform code, but 
> adding [[maybe_unused]] in the header file is straight forward.
I think it would be better to put #if PLATFORM(COCOA) around member variables 
like this because I don’t think [[maybe_unused]] will remove the additional 
byte in the structure.  Otherwise we’re just wasting memory.

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Use of [[maybe_unused]]

[Suzuki, Basuke]

Hi WebKit-dev,

I'm asking about whether we can use C++ 17's new attribute [[maybe_unused]]. 
These are situations I cannot solve well with UNUSED_PARAM().

Case 1: The function parameter is only used in RELEASE_LOG macro which is empty 
in some platform
https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp#L105

m_workQueue->dispatch([this, host = host.isolatedCopy(), sessionID, 
callback = WTFMove(callback)] () mutable {
...
RELEASE_LOG_IF_ALLOWED(sessionID, "query - Ran successfully. Result = 
%s", (foundHost ? "true" : "false"));
...
});

`sessionID` is used in RELEASE_LOG_IF_ALLOWED() and we have empty 
implementation of RELEASE_LOG() so that it's ended up with unused parameter 
warning of sessionID. We can add UNUSED_PARAM(sessionID) in this case, but 
[[maybe_unused]] is more correct choice to describe the code because sessionID 
is actually used.

I also tried to replace empty RELEASE_LOG macro with something to use every 
parameter marked used somehow, but the trial just failed.

Case 2: The member variable is just used by specific port
https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkLoadChecker.h#L155

bool m_isHTTPSUpgradeEnabled { false };

This member variable is only used in COCOA port. 
(https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp#L203)

We can add UNUSED_PARAM(isHTTPSUpgradeEnabled) in our platform code, but adding 
[[maybe_unused]] in the header file is straight forward.

Compiler support of this attribute seems okay for any WebKit build. I've done 
quick check on Godbolt. https://godbolt.org/z/w47XXn


-
Basuke Suzuki
SONY PlayStation

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Step 1 of Plugin removal: Deleting NPAPI (and thus Flash support)

[Dean Jackson]

> On 13 Jan 2020, at 20:14, Carlos Garcia Campos  wrote:
> 
> El lun, 13-01-2020 a las 05:30 +1100, Dean Jackson escribió:
>> Dear Non-Apple ports,
>> 
>> Running Flash has been more difficult over the past few years as part
>> of a (semi-) coordinated effort by browsers and Adobe. The plan is to
>> remove support for Flash + NPAPI by the end of this year. See the
>> links below. 
>> 
>> I'd like to remove our NPAPI code soon, but I want to make sure the
>> other ports are ok with this. Please speak up if you have a reason to
>> keep it in.
> 
> WPE has never supported NPAPI plugins and the GTK port removed the
> support for GTK2 plugins (flash) already in our current stable version.
> Plugins not using GTK at all (or using GTK3) are still supported by GTK
> port (some of them only under X11, though). I'm ok with removing the
> NPAPI plugins support in the GTk port, but we are at the end of the
> release cycle, so I prefer if we remove the feature right after we
> branch for the next stable version (scheduled for the 1st February). I
> could even branch earlier if needed.

Waiting until February is totally ok with me. Good luck with your release.

Dean

> 
>> [Note that we will still have some plugin code e.g. our internal
>> PDFPlugin, just no support for externally installed plugins]
>> 
>> Dean
>> 
>> * Adobe's end of life for Flash - 
>> https://theblog.adobe.com/adobe-flash-update/
>> * Chrome removing Flash support by end of 2020 - 
>> https://sites.google.com/a/chromium.org/dev/flash-roadmap
>> * Google removing support for Flash by end of 2020 - 
>> https://www.blog.google/products/chrome/saying-goodbye-flash-chrome/
>> * Mozilla removing NPAPI by end of 2020 - 
>> https://developer.mozilla.org/en-US/docs/Plugins/Roadmap
>> * Mozilla only uses NPAPI for Flash - 
>> https://blog.mozilla.org/futurereleases/2015/10/08/npapi-plugins-in-firefox/
>> 
> 
> Thanks!
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 


smime.p7s
Description: S/MIME cryptographic signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Step 1 of Plugin removal: Deleting NPAPI (and thus Flash support)

[Carlos Garcia Campos]

El lun, 13-01-2020 a las 05:30 +1100, Dean Jackson escribió:
> Dear Non-Apple ports,
> 
> Running Flash has been more difficult over the past few years as part
> of a (semi-) coordinated effort by browsers and Adobe. The plan is to
> remove support for Flash + NPAPI by the end of this year. See the
> links below. 
> 
> I'd like to remove our NPAPI code soon, but I want to make sure the
> other ports are ok with this. Please speak up if you have a reason to
> keep it in.

WPE has never supported NPAPI plugins and the GTK port removed the
support for GTK2 plugins (flash) already in our current stable version.
Plugins not using GTK at all (or using GTK3) are still supported by GTK
port (some of them only under X11, though). I'm ok with removing the
NPAPI plugins support in the GTk port, but we are at the end of the
release cycle, so I prefer if we remove the feature right after we
branch for the next stable version (scheduled for the 1st February). I
could even branch earlier if needed.

> [Note that we will still have some plugin code e.g. our internal
> PDFPlugin, just no support for externally installed plugins]
> 
> Dean
> 
> * Adobe's end of life for Flash - 
> https://theblog.adobe.com/adobe-flash-update/
> * Chrome removing Flash support by end of 2020 - 
> https://sites.google.com/a/chromium.org/dev/flash-roadmap
> * Google removing support for Flash by end of 2020 - 
> https://www.blog.google/products/chrome/saying-goodbye-flash-chrome/
> * Mozilla removing NPAPI by end of 2020 - 
> https://developer.mozilla.org/en-US/docs/Plugins/Roadmap
> * Mozilla only uses NPAPI for Flash - 
> https://blog.mozilla.org/futurereleases/2015/10/08/npapi-plugins-in-firefox/
> 

Thanks!

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev