Re: [webkit-dev] Prefix naming scheme for DOM-exposed functions
On Fri, Dec 7, 2012 at 1:36 PM, Maciej Stachowiak wrote: > > Would this involve creating a bindingsFoo() for every method foo() that is > exposed to bindings? For example, would we have to add > XMLHttpRequest::bindingsSend(), even though there's no real need for a > special internal XMLHttpRequest::send()? Would getters and setters that map > to JavaScript properties (but which do not reflect markup attributes) also > need a bindings... version? For example, would we need > HTMLMediaElement::bindingsMuted() and HTMLMediaElement::bindingsSetMuted() > to wrap the regular muted() and setMuted()? > I see no internal clients for HTMLMediaElement::muted()/setMuted() so there would be no need for internal versions for it. I suspect that in many cases we would at most need an internal accessor (which could then properly follow our naming conventions HTMLMediaElement::isMuted()). > If the answer to these questions is "yes", then I think this is too much > complexity tax on all exposed methods and properties to make up for the > benefit. It's likely only a minority of methods where it's highly desirable > to have a specialized version for internal use. > Probably the only way to find out is try doing it and seeing how the patch ends up looking like. > As a side note, I don't see how this would address the concrete example > given, that of firstElementChild likely becoming more efficient than > firstChild. If we add bindingsFirstChild() and bindingsFirstElementChild(), > how does this help WebCore developers know that they should use the > internal firstElementChild() instead of the internal firstChild()? I expect > both have to exist, because there really are cases where you need to > traverse the whole DOM, not just elements, and even if we were to convert, > firstElementChild() is not a drop-in replacement. > This would create more freedom for our internal document tree to diverge from how it looks like through the DOM api. For example internal firstChild() that returns Node* may no longer make sense at all if we eliminate Text nodes. antti > It also seems to me that internal methods that do the exact same thing as > a bindings...() version but lack an ExceptionCode parameter, we'll still > want to avoid excess code duplication, in some cases of tricky algorithms. > I would not want a second copy of ContainerNode::insertBefore() and its > helper methods that replaces exception checking with preflight checks that > return false without setting an exeption code (I don't think you can just > skip the checks entirely or you'd make a method that is extremely dangerous > to call if you have not met very complex preconditions). > > I do agree with the goal of having efficient internal interfaces that are > not constrained by what is exposed to the Web, but a blanket introduction > of methods just for bindings does not seem like a good way to get there. > > Possible alternatives: > - Use something in the IDL to call a bindings... variant only in cases > where we know there is a materially better internal method. > - Use the style checker to ban calling select exposed methods from > hand-written WebCore code, and give the corresponding internal methods > different names. > > These approaches could achieve the goals described for critical DOM > methods without having to infect things like XHR::send() or > HTMLMediaElement::setMuted(). > > Regards, > Maciej > > On Dec 7, 2012, at 9:27 AM, Darin Adler wrote: > > > Hi folks. > > > > Many of the APIs designed for use in the DOM are not efficient for use > inside WebKit, or have designs that are better for JavaScript than for C++. > Antti Koivisto and I have been discussing how to best communicate this to > WebKit contributors so they don’t end up using inefficient idioms just > because they are familiar with them from use in JavaScript code on > websites. So far, our best idea for this is to add a prefix to function > names that indicate they are functions for use by the bindings machinery. > Thus, a function like appendChild would get a new name: > > > >void bindingsAppendChild(Node*, ExceptionCode&); > > > > The internal function that’s used to add a child node would be designed > for the best clarity, ease of use, and efficiency within WebKit > implementation code, even when that does not match up with the DOM > standard. And could be refactored over time as WebKit design changes > without affecting the bindings. > > > > - It’s not clear what the best prefix is. I don’t like the prefix “dom”, > since it’s a lowercased acronym and an overloaded not all that precise > term. The prefix “bindings” is sort of silly because these functions are > not themselves “bindings”, rather they are the non-language-specific > functions designed to be bound to each language. However, I do like the > idea of a brief non-acronym word. So, still looking for a great prefix. > > > > - When appropriate, these exposed functions can be short inline > functions that turn around an
Re: [webkit-dev] Prefix naming scheme for DOM-exposed functions
(sending from correct address)
Come late here. I basically support Darin's point.
Here is some addition:
As Maciej said, we don't need to do this for all API. We can do this by
opt-in basis, as an optimization.
Since we have [implementedAs] IDL attribute already, we can even start this
today once consensus on prefix naming is made (I'd vote for "binding"
prefix.)
On taking error check apart, another possible idea is to get more help from
our code generators.
For example, we could add "Check" C++ function which returns an
ExceptionCode, then we could let generated code call it appropriately.
// IDL will look like this:
[IfCheckFail=ReturnNull] readonly attribute Node firstChild;
// generated code will look like this.
ExceptionCode ec = impl->firstChildCheck();
if (ec)
return jsNull();
return toJS(impl->firstCihld());
// implementation could be
Node* ContainerNode::firstChild() const {
ASSERT(!firstChildCheck());
return m_firstChild;
}
I expect that there are only a few major patterns for handling and
reporting errors.
So we can also tell such patterns to the generator.
// Another IDL example:
[ExceptionIfNull=InvalidStateErorr] readonly attribute Node startContainer
getter raises(DOMException);
// generated code will be something like...
RefPtr return = impl->startContainer();
if (!return) {
throwException();
return jsNull();
}
return toJS(return);
This "IDL annotations plus optional C++ helpers" approach isn't as clean
(or simple) as uniform prefixing.
But it could result less hand-written C++ code and achieve same performance
benefit as original proposal.
Its opt-in nature will also help us try these ideas incrementally.
On the other hand, this could possibly result more complex, hard to
understand code backed by half-baked magic, if we do this badly.
I don't think this becomes that complex though.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Prefix naming scheme for DOM-exposed functions
On Dec 7, 2012, at 12:28 PM, Elliott Sprehn wrote: > This seems like it would introduce bugs and make maintaining the DOM harder > since we'd need to duplicate logic. We should not needlessly duplicate logic. If the change causes us to copy and paste code, we’re doing it wrong. > Right now we have appendChild() and parserAppendChild(), and using > parserAppendChild() for anything not in the parser introduces web observable > bugs and changes in behavior. That seems like an overgeneralization. The fact that parser-suitable behavior is obtained by calling a separate function rather than by passing some special argument to appendChild doesn’t seem to reflect on this proposal one way or another. Using the wrong function is going to give the wrong result, in both directions. Using appendChild instead of parserAppendChild in the parser would also introduce web observable bugs and change behavior. A problem with the web-exposed functions is that their names are dictated by the standards and existing code on the web, and so can easily to be misleading for people writing code within WebKit. We have had problems in the past because code doing internal things calls temptingly named web-exposed functions and gets unwanted behavior. One recent example is that Eric Seidel and I suspect, but have not yet proven, that the change to use appendChild instead of parserAppendChild in https://bugs.webkit.org/show_bug.cgi?id=104040 introduced web observable problems. > We also only have tests for the web exposed methods, unless you're suggesting > we add C++ unit tests too? :) A fair point. I think we’d still have sufficient test coverage without being able to unit test each function. Currently we have thousands if not tens of thousands of non-web-exposed functions and we test them indirectly through the web-exposed functions, or expose them via internals. The same would be true of these new functions. I believe we do have C++ unit tests, although we also test as much as possible with the JavaScript ones. > Can you give an example of what you want to change in the non web exposed > appendChild() ? For internal use, we’d have an appendChild-like function on ContainerNode, not Node, and that takes an Element* rather than a Node*. The web-exposed appendChild needs logic that the internal function would not, to handle failed attempts to append a child to non-container nodes and to handle text nodes. We’d have a separate function for adding text content to a node that should be used instead of creating a text node and then calling appendChild. The bindings-exposed function would call through to the internal functions. I’m not sure, but I also suspect that our internal appendChild-like function would probably be a single function that encompasses both insertBefore and appendChild. There are many common web-exposed functions and attributes we don’t ever want to be called internally. Examples from Node.idl are nodeType, considerably slower than the internal techniques to ask the same questions, childNodes, an inefficient way to iterate the children of a node, attributes, an inefficient way to work with attributes, and ownerDocument, a slower version of document that has to do an extra branch because it needs to return 0 when called on a document. Don’t even get me started on isSameNode. On Dec 7, 2012, at 1:36 PM, Maciej Stachowiak wrote: > Would this involve creating a bindingsFoo() for every method foo() that is > exposed to bindings? For example, would we have to add > XMLHttpRequest::bindingsSend(), even though there's no real need for a > special internal XMLHttpRequest::send()? We’d rename to the prefixed name, rather than adding a new second function. There is no internal WebKit use of XMLHttpRequest::send except to implement other overloads of XMLHttpRequest::send. > Would getters and setters that map to JavaScript properties (but which do not > reflect markup attributes) also need a bindings... version? For example, > would we need HTMLMediaElement::bindingsMuted() and > HTMLMediaElement::bindingsSetMuted() to wrap the regular muted() and > setMuted()? Yes, we would have to either rename or have two functions. I can see that on the media elements, with a broad interface, that could be a problem. It does seem that setMuted is called in two or three places internally in WebKit code. > I think this is too much complexity tax on all exposed methods and properties > to make up for the benefit. It's likely only a minority of methods where it's > highly desirable to have a specialized version for internal use. Perhaps. On a class like Node it looks like it’s a majority of methods, but on a class like HTMLMediaElement it looks like a minority. > How does this help WebCore developers know that they should use the internal > firstElementChild() instead of the internal firstChild()? I expect both have > to exist, because there really are cases where you need to t
Re: [webkit-dev] Prefix naming scheme for DOM-exposed functions
Would this involve creating a bindingsFoo() for every method foo() that is exposed to bindings? For example, would we have to add XMLHttpRequest::bindingsSend(), even though there's no real need for a special internal XMLHttpRequest::send()? Would getters and setters that map to JavaScript properties (but which do not reflect markup attributes) also need a bindings... version? For example, would we need HTMLMediaElement::bindingsMuted() and HTMLMediaElement::bindingsSetMuted() to wrap the regular muted() and setMuted()? If the answer to these questions is "yes", then I think this is too much complexity tax on all exposed methods and properties to make up for the benefit. It's likely only a minority of methods where it's highly desirable to have a specialized version for internal use. As a side note, I don't see how this would address the concrete example given, that of firstElementChild likely becoming more efficient than firstChild. If we add bindingsFirstChild() and bindingsFirstElementChild(), how does this help WebCore developers know that they should use the internal firstElementChild() instead of the internal firstChild()? I expect both have to exist, because there really are cases where you need to traverse the whole DOM, not just elements, and even if we were to convert, firstElementChild() is not a drop-in replacement. It also seems to me that internal methods that do the exact same thing as a bindings...() version but lack an ExceptionCode parameter, we'll still want to avoid excess code duplication, in some cases of tricky algorithms. I would not want a second copy of ContainerNode::insertBefore() and its helper methods that replaces exception checking with preflight checks that return false without setting an exeption code (I don't think you can just skip the checks entirely or you'd make a method that is extremely dangerous to call if you have not met very complex preconditions). I do agree with the goal of having efficient internal interfaces that are not constrained by what is exposed to the Web, but a blanket introduction of methods just for bindings does not seem like a good way to get there. Possible alternatives: - Use something in the IDL to call a bindings... variant only in cases where we know there is a materially better internal method. - Use the style checker to ban calling select exposed methods from hand-written WebCore code, and give the corresponding internal methods different names. These approaches could achieve the goals described for critical DOM methods without having to infect things like XHR::send() or HTMLMediaElement::setMuted(). Regards, Maciej On Dec 7, 2012, at 9:27 AM, Darin Adler wrote: > Hi folks. > > Many of the APIs designed for use in the DOM are not efficient for use inside > WebKit, or have designs that are better for JavaScript than for C++. Antti > Koivisto and I have been discussing how to best communicate this to WebKit > contributors so they don’t end up using inefficient idioms just because they > are familiar with them from use in JavaScript code on websites. So far, our > best idea for this is to add a prefix to function names that indicate they > are functions for use by the bindings machinery. Thus, a function like > appendChild would get a new name: > >void bindingsAppendChild(Node*, ExceptionCode&); > > The internal function that’s used to add a child node would be designed for > the best clarity, ease of use, and efficiency within WebKit implementation > code, even when that does not match up with the DOM standard. And could be > refactored over time as WebKit design changes without affecting the bindings. > > - It’s not clear what the best prefix is. I don’t like the prefix “dom”, > since it’s a lowercased acronym and an overloaded not all that precise term. > The prefix “bindings” is sort of silly because these functions are not > themselves “bindings”, rather they are the non-language-specific functions > designed to be bound to each language. However, I do like the idea of a brief > non-acronym word. So, still looking for a great prefix. > > - When appropriate, these exposed functions can be short inline functions > that turn around and call internal functions. > > - These functions aren’t needed at all to implement reflected content > attributes. Hooray! > > - So far my best idea on how to stage this is to new inlines without cutting > the bindings over to them. Then cut the bindings generation script over, then > remove and refactor the various unneeded underlying functions. Other ways to > stage this would be add an attribute so we can can switch a class or a > function at a time over to the new naming scheme, but base classes could make > that process challenging and needlessly complex. > > - We don’t want to use ExceptionCode& arguments much in internal functions. > They lead both to confusing code and to inefficiency, and I think we can do > much better witho
Re: [webkit-dev] Prefix naming scheme for DOM-exposed functions
This seems like it would introduce bugs and make maintaining the DOM harder since we'd need to duplicate logic. Right now we have appendChild() and parserAppendChild(), and using parserAppendChild() for anything not in the parser introduces web observable bugs and changes in behavior. We also only have tests for the web exposed methods, unless you're suggesting we add C++ unit tests too? :) Can you give an example of what you want to change in the non web exposed appendChild() ? On Fri, Dec 7, 2012 at 9:27 AM, Darin Adler wrote: > Hi folks. > > Many of the APIs designed for use in the DOM are not efficient for use > inside WebKit, or have designs that are better for JavaScript than for C++. > Antti Koivisto and I have been discussing how to best communicate this to > WebKit contributors so they don’t end up using inefficient idioms just > because they are familiar with them from use in JavaScript code on > websites. So far, our best idea for this is to add a prefix to function > names that indicate they are functions for use by the bindings machinery. > Thus, a function like appendChild would get a new name: > > void bindingsAppendChild(Node*, ExceptionCode&); > > The internal function that’s used to add a child node would be designed > for the best clarity, ease of use, and efficiency within WebKit > implementation code, even when that does not match up with the DOM > standard. And could be refactored over time as WebKit design changes > without affecting the bindings. > > - It’s not clear what the best prefix is. I don’t like the prefix “dom”, > since it’s a lowercased acronym and an overloaded not all that precise > term. The prefix “bindings” is sort of silly because these functions are > not themselves “bindings”, rather they are the non-language-specific > functions designed to be bound to each language. However, I do like the > idea of a brief non-acronym word. So, still looking for a great prefix. > > - When appropriate, these exposed functions can be short inline functions > that turn around and call internal functions. > > - These functions aren’t needed at all to implement reflected content > attributes. Hooray! > > - So far my best idea on how to stage this is to new inlines without > cutting the bindings over to them. Then cut the bindings generation script > over, then remove and refactor the various unneeded underlying functions. > Other ways to stage this would be add an attribute so we can can switch a > class or a function at a time over to the new naming scheme, but base > classes could make that process challenging and needlessly complex. > > - We don’t want to use ExceptionCode& arguments much in internal > functions. They lead both to confusing code and to inefficiency, and I > think we can do much better without them. But they are still probably a > good efficient way to indicate the need for an exception to the JavaScript > binding. We’d eliminate ASSERT_NO_EXCEPTION as part of this. > > - This will be particularly helpful for future optimizations, such as one > we are contemplating that will make currently-heavily-used functions such > as firstChild more expensive, and currently-lightly-used functions such as > firstElementChild cheaper. We need a way to rename such things and find > internal callers and prevent people from accidentally undoing that effort > as they do additional WebKit work. > > -- Darin > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Prefix naming scheme for DOM-exposed functions
Sounds like a good idea in general. On Fri, Dec 7, 2012 at 9:27 AM, Darin Adler wrote: > > Many of the APIs designed for use in the DOM are not efficient for use > inside WebKit, or have designs that are better for JavaScript than for C++. > Antti Koivisto and I have been discussing how to best communicate this to > WebKit contributors so they don’t end up using inefficient idioms just > because they are familiar with them from use in JavaScript code on > websites. So far, our best idea for this is to add a prefix to function > names that indicate they are functions for use by the bindings machinery. > Thus, a function like appendChild would get a new name: > > void bindingsAppendChild(Node*, ExceptionCode&); > > The internal function that’s used to add a child node would be designed > for the best clarity, ease of use, and efficiency within WebKit > implementation code, even when that does not match up with the DOM > standard. And could be refactored over time as WebKit design changes > without affecting the bindings. > > - It’s not clear what the best prefix is. I don’t like the prefix “dom”, > since it’s a lowercased acronym and an overloaded not all that precise > term. The prefix “bindings” is sort of silly because these functions are > not themselves “bindings”, rather they are the non-language-specific > functions designed to be bound to each language. However, I do like the > idea of a brief non-acronym word. So, still looking for a great prefix. > How about publicAppendChild, exportedAppendChild, apiAppendChild, exposedAppendChild, or jsAppendChild? The last one has a benefit of us being able to add alternative functions like objcAppendChild when Obj-C and JS interfaces are different. i.e. non-JS bindings falls back to use jsAppendChild by default. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Prefix naming scheme for DOM-exposed functions
Hi folks. Many of the APIs designed for use in the DOM are not efficient for use inside WebKit, or have designs that are better for JavaScript than for C++. Antti Koivisto and I have been discussing how to best communicate this to WebKit contributors so they don’t end up using inefficient idioms just because they are familiar with them from use in JavaScript code on websites. So far, our best idea for this is to add a prefix to function names that indicate they are functions for use by the bindings machinery. Thus, a function like appendChild would get a new name: void bindingsAppendChild(Node*, ExceptionCode&); The internal function that’s used to add a child node would be designed for the best clarity, ease of use, and efficiency within WebKit implementation code, even when that does not match up with the DOM standard. And could be refactored over time as WebKit design changes without affecting the bindings. - It’s not clear what the best prefix is. I don’t like the prefix “dom”, since it’s a lowercased acronym and an overloaded not all that precise term. The prefix “bindings” is sort of silly because these functions are not themselves “bindings”, rather they are the non-language-specific functions designed to be bound to each language. However, I do like the idea of a brief non-acronym word. So, still looking for a great prefix. - When appropriate, these exposed functions can be short inline functions that turn around and call internal functions. - These functions aren’t needed at all to implement reflected content attributes. Hooray! - So far my best idea on how to stage this is to new inlines without cutting the bindings over to them. Then cut the bindings generation script over, then remove and refactor the various unneeded underlying functions. Other ways to stage this would be add an attribute so we can can switch a class or a function at a time over to the new naming scheme, but base classes could make that process challenging and needlessly complex. - We don’t want to use ExceptionCode& arguments much in internal functions. They lead both to confusing code and to inefficiency, and I think we can do much better without them. But they are still probably a good efficient way to indicate the need for an exception to the JavaScript binding. We’d eliminate ASSERT_NO_EXCEPTION as part of this. - This will be particularly helpful for future optimizations, such as one we are contemplating that will make currently-heavily-used functions such as firstChild more expensive, and currently-lightly-used functions such as firstElementChild cheaper. We need a way to rename such things and find internal callers and prevent people from accidentally undoing that effort as they do additional WebKit work. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
