On Thu, Mar 8, 2018 at 5:56 PM, Kris Maglione <kmagli...@mozilla.com> wrote:

> On Thu, Mar 08, 2018 at 04:41:38PM -0800, Bobby Holley wrote:
>
>> On Thu, Mar 8, 2018 at 3:06 PM, Kris Maglione <kmagli...@mozilla.com>
>> wrote:
>>
>>> That said, if we're worried about binary size becoming an issue for
>>> internal interfaces, there are things we can do to reduce the code size
>>> of
>>> bindings. Particularly if we're willing to eat the performance costs.
>>>
>>
>> WebIDL bindings are optimized for speed above all else, and that shouldn't
>> have to change to mitigate overuse.
>>
>
> My point is that if we're deciding that we need to make a trade-off
> between speed and compiled binary size, I think that we're better off doing
> that by changing how we generate the bindings for interfaces that we decide
> are not performance sensitive than deciding to use XPIDL for them. If
> nothing else, it makes it easier for us to make the change for a particular
> interface, or to change our mind.
>

To be sure I understand: you're proposing what Cameron proposed, which is
to support two separate modes in the WebIDL bindings ("fast" and
"compact")? And the "compact" mode would use general hooks and interpret
arguments on-the-fly like XPConnect does?

Doing so would be an enormous amount of work (measured in engineer-years),
and the result would likely have lots of bugs in the corner cases. Asking
people to be thoughtful about their usage of WebIDL vs XPIDL is much more
attractive.

At any rate, I don't expect us to convert anywhere near all of our XPIDL
>>> interfaces to WebIDL. A lot of them don't need to be exposed to JS at
>>> all.
>>> A lot of those should still go away, but they don't need WebIDL bindings,
>>> just concrete native classes. And a lot of the rest are little-enough
>>> used
>>> that I can't see anyone spending the effort on converting them.
>>>
>>
>>
>> I am basically worried about two things:
>> (1) Wholescale conversions of big interfaces in the name of cleanup and
>> ergonomics. See bug 1341546 for an example.
>>
>
> Heh. It's interesting that you bring that interface up, because I've been
> thinking for a long time that it's one of the most obvious examples of
> something we should convert to WebIDL. We use it all over the place, and
> it's one of the places that I see XPConnect overhead turn up most for in
> profiles.
>

I just looked at the first 10 methods/attributes on that interface. None of
them are remotely performance-sensitive, and several are test-only. If we
see certain methods on it show up in profiles, we should move those methods
to WebIDL, rather than converting things wholesale per-interface.


>
> I'm not entirely sure whether a review gate is necessary. But at the very
>> least, I want to establish a consensus around that we should only use
>> WebIDL to expose internal interfaces if one of the following applies:
>> (A) The API is likely to be called hundreds of times under normal browser
>> execution.
>> (B) The API is associated with a DOM object, and thus adding it
>> [ChromeOnly] to that interface is particularly convenient.
>> (C) The API uses complex arguments like promises that XPIDL doesn't handle
>> in a nice way.
>>
>> Opinions?
>>
>
> I don't really have a problem with these criteria. That's more or less
> what I consider when deciding how to implement bindings.
>
> But I'd really rather we didn't have to make this trade-off. There's no
> fundamental reason WebIDL bindings have to have more code size overhead
> than XPIDL bindings.


There totally is.

There are basically two ways to do JS<->C++ bindings: generating explicit
stubs for each method, or using a single generic stub with compact type
information to convert things on the fly.

Gecko originally did the former (MIDL). For various reasons (including code
size), we then we switched everything to the latter (XPConnect/XPIDL). But
the latter ran into two problems, which caused us to flip-flop and
reimplement a codegen setup for the DOM:
(A) Doing everything on-the-fly was really slow.
(B) It was increasingly difficult to properly handle the complex and
expressive types making their way into WebIDL.

So even if we didn't care about (A), and even if we were willing to spend
the time to make an XPConnect-like backend for WebIDL, we'd run straight
into the same problems trying to support the trickier bits of WebIDL.

The implementation details are almost completely separated from the
> consumers, and if at some point we decide the overhead is becoming a
> problem and we need to make a trade-off, we can always change the
> implementation that we use for interfaces we think are not performance
> critical.


The WebIDL codegen bindings took years for a number of our best engineers
to build. I don't see a case here for taking on more work of that magnitude
when the alternative is so straightforward.


On Thu, Mar 8, 2018 at 3:16 PM, Stuart Philp <sph...@mozilla.com> wrote:
>
> Generally I think we’d take performance and memory wins over installer
>> size, but we monitor all three and if installer size grows (gradually) by
>> an uncomfortable amount we ought to make a call on the trade off. We can
>> bring it to product should that happen.
>>
>>
> The problem is precisely that it's gradual - a few kilobytes at a time,
> certainly nothing to trigger our alerts. Waiting for it all to pile up and
> then launching a herculean effort to move things _back_ to XPIDL would be a
> huge waste of time, which is why I'm trying to address the problem now.
>
>
> On Thu, Mar 8, 2018 at 2:01 PM, Kris Maglione <kmagli...@mozilla.com>
>>
>>> wrote:
>>>
>>> It is now possible[1] to create chrome-only WebIDL interfaces in the
>>>
>>>> dom/chrome-webidl/ directory that do not require review by DOM peers
>>>> after
>>>> every change. If you maintain an internal performance-sensitive XPIDL
>>>> interface, or are considering creating one, I'd encourage you to
>>>> consider
>>>> migrating it to WebIDL.
>>>>
>>>> Some caveats to keep in mind:
>>>>
>>>> - Interfaces in this directory must be annotated with [ChromeOnly].
>>>> Dictionaries, however, can be included without any special annotations.
>>>>
>>>> - If you are new to writing WebIDL files, I'd still encourage you to ask
>>>> a
>>>> DOM peer to review at least your initial check-in.
>>>>
>>>> - Please make sure that you do not attempt to expose any of the
>>>> interface
>>>> or dictionary types defined in these WebIDL files to web contexts,
>>>> through
>>>> interfaces defined in dom/webidl/. Doing so would require (and fail) DOM
>>>> peer review, in any case, but please think ahead.
>>>>
>>>> Thanks.
>>>>
>>>> - Kris
>>>>
>>>> [1]: As of bugs 1443317 and 1442931
>>>>
>>>>
>>>
-- 
> Kris Maglione
> Senior Firefox Add-ons Engineer
> Mozilla Corporation
>
> It is practically impossible to teach good programming style to
> students that have had prior exposure to Basic; as potential
> programmers they are mentally mutilated beyond hope of regeneration.
>         --Edsger W. Dijkstra
>
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to