Re: [webkit-dev] Reducing / removing use of Makefile based DerivedSources.make

2020-10-20 Thread Keith Rollin
> On Oct 20, 2020, at 10:51, Andy Estes  wrote:
> 
>> On Oct 18, 2020, at 11:01, Sam Weinig  wrote:
>> 
>> One direct benefit of moving away from DerivedSources.make would likely be 
>> (I say likely, as the details of how it works out are far from certain in my 
>> mind) removing at least one place that a IDL file needs to be listed as we 
>> would not need to explicitly the list the IDL file in DerivedSources.make, 
>> and would only need to ensure it was in some *input.xcfilelist.
> 
> The way I imagine this working is that the existing .idl file references in 
> the (e.g., WebCore) project would become members in a target with a custom 
> build rule for “*.idl”. Target membership is itself another list inside the 
> .xcodeproj file, just one you manage through checkboxes in Xcode (and can be 
> done reasonably quickly as part of adding a new file reference).
> 
> Whether or not this actually creates one fewer “place a IDL file needs to be 
> listed” (which I think is a worthwhile goal), I think you should still remove 
> the Makefile build phases from Xcode for the reasons you originally cited.
> 
> Andy

Overall, this sounds like an awesome endeavor. The initial reasons for using 
Makefiles were sound ones (as Darin described), but there are also the 
incumbent issues, such as maintainability and efficiency.

I wasn’t clear on what the alternative to Makefiles would look like, though. 
The options that occur to me are:

* Custom build rules, as Andy describes. This seems to make sense, but I wonder 
if it is flexible enough for all the cases we might want to put it through. Our 
DerivedSources.make files process many types of files. I haven’t checked, but I 
could imagine that not all of these files could be uniquely distinguished by 
their suffixes. That is, there might be one set of *.foo files that should be 
processed one way, and another set that should processed another way. In those 
cases, I guess some suffixes could be changed to accommodate the different 
processing.

* Using a set of Run Script build phases. That is, have a Run Script phase for 
processing a specific set of *.idl files, another for processing one set of 
*.foo files, a third for processing a different specific set of *.foo files, 
and so on. I’m against this approach, though. While it offers lots of 
flexibility, it’s not very maintainable. It would mean having to keep the set 
of script inputs and outputs up-to-date, something that is fragile, even with 
the help of the generate-xcfilelists utility script.

Of these two, the one based on custom build rules seems to make the most sense. 
One need only add a file to an Xcode project and possibly a CMake file. Of 
note, the dreaded .xcfilelists are not affected (and, if this project can be 
taken to its extreme, we can get rid of the DerivedSources*.xcfilelist files). 
I do worry about some efficiency issues, though. For instance, one part of 
DerivedSources.make processes Platform.h (and all its children) in order to 
extract some configuration information. That processing is done once per 
DerivedSources.make invocation. If we move to a model where a script is invoked 
once per *.idl file, then the overhead is now multiplied by the number of *.idl 
files. We should watch for that. (Perhaps your recent 
configuration/preferences/settings changes obviate the need to pre-process 
Platform.h all the time?)

Still, I’m open to a solution based on a single IDL.txt file that can be shared 
by Xcode and CMake. It all depends on the details, the resulting solution, and 
the trade-offs it makes.

Regarding Xcode vs. CMake, I certainly understand the reasons people would like 
to see us move solely to CMake. But, for now, the plan is to see if we can get 
Apple-OS ports to build with CMake at all. Replacing Xcode and xcodebuild is a 
follow-on project with an interesting set of issues. I set up #cmake on Slack 
to provide a place where we can talk about this for anyone who’s interested.

— Keith

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


Re: [webkit-dev] Switching to XCBuild for Apple builds

2020-04-01 Thread Keith Rollin
> On Apr 1, 2020, at 06:21, Frédéric Wang  wrote:
> 
> On 31/03/2020 08:42, Keith Rollin wrote:
> 
>> For several years now, Xcode has included two build systems: the “legacy” 
>> build system and the newer “XCBuild” system (see, for example, 
>> <https://developer.apple.com/videos/play/wwdc2018/408/> and 
>> <https://developer.apple.com/videos/play/wwdc2018/415>, as well as 
>> <https://medium.com/xcblog/xcode-new-build-system-for-speedy-swift-builds-c39ea6596e17>
>>  and 
>> <https://medium.com/xcblog/five-things-you-must-know-about-xcode-10-new-build-system-41676cd5fd6c>).
>> 
>> WebKit is finally in a position to move to the new build system. However, 
>> this switch is not without consequences. In particular, WebKit has a 
>> complicated build process and requires new XCBuild features only found in 
>> Xcode 11.4. If you build from within the IDE, you will need Xcode 11.4 or 
>> later. Pre-releases have been available for some time on 
>> developer.apple.com, but the final version of Xcode 11.4 was posted to the 
>> Mac App Store last week on March 24, 2020.
> 
> Out of curiosity, what will be the benefits brought by XCBuild for
> WebKit developers? (sorry for asking but I don't have time to watch the
> videos and skimming over the blog post, it seems it's mostly performance
> improvement for swift compilation?)


I created <https://bugs.webkit.org/show_bug.cgi?id=209890> and responded to 
your question there.


>> When building from the command line, the conditions are less strict. By 
>> default, builds are performed with XCBuild if the `build-webkit` script or 
>> the makefiles detect that Xcode 11.4 or later is installed. If not, builds 
>> are performed with the legacy build system. We can be flexible here because 
>> the build process is controlled by these scripts and makefiles, allowing the 
>> build process to determine the Xcode version before invoking `xcodebuild`. 
>> There is no such opportunity when building within the Xcode IDE, and so the 
>> build requirements are stricter.
>> 
>> I’ve made the changes to switch over to XCBuild but I haven’t yet posted 
>> them. I want to first find out if this move will cause any issues for 
>> developers who — for whatever reason — need to stay on old versions of 
>> Xcode. If so, please let me know.
> So I'm not sure I understand but concretely, am I already using the new
> system if I use the build-script, I've upgraded to XCode 11.4 and
> rebuilt WebKit from scratch? How can I know which build system I'm using?
> 
> Or do you mean you have a patch for WebKit that is pending to land? If
> so, what's the bug number?


The patch was just on my local system. I wanted to make sure I’d covered any 
issues that were turned up in response to my email before posting. But I’ve now 
created the bug noted above and added the patch to it.

— Keith

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


[webkit-dev] Switching to XCBuild for Apple builds

2020-03-31 Thread Keith Rollin
Hi all,

For several years now, Xcode has included two build systems: the “legacy” build 
system and the newer “XCBuild” system (see, for example, 
<https://developer.apple.com/videos/play/wwdc2018/408/> and 
<https://developer.apple.com/videos/play/wwdc2018/415>, as well as 
<https://medium.com/xcblog/xcode-new-build-system-for-speedy-swift-builds-c39ea6596e17>
 and 
<https://medium.com/xcblog/five-things-you-must-know-about-xcode-10-new-build-system-41676cd5fd6c>).

WebKit is finally in a position to move to the new build system. However, this 
switch is not without consequences. In particular, WebKit has a complicated 
build process and requires new XCBuild features only found in Xcode 11.4. If 
you build from within the IDE, you will need Xcode 11.4 or later. Pre-releases 
have been available for some time on developer.apple.com, but the final version 
of Xcode 11.4 was posted to the Mac App Store last week on March 24, 2020.

When building from the command line, the conditions are less strict. By 
default, builds are performed with XCBuild if the `build-webkit` script or the 
makefiles detect that Xcode 11.4 or later is installed. If not, builds are 
performed with the legacy build system. We can be flexible here because the 
build process is controlled by these scripts and makefiles, allowing the build 
process to determine the Xcode version before invoking `xcodebuild`. There is 
no such opportunity when building within the Xcode IDE, and so the build 
requirements are stricter.

I’ve made the changes to switch over to XCBuild but I haven’t yet posted them. 
I want to first find out if this move will cause any issues for developers who 
— for whatever reason — need to stay on old versions of Xcode. If so, please 
let me know.

— Keith Rollin

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


Re: [webkit-dev] Requesting feedback about EWS comments on Bugzilla bugs

2019-06-17 Thread Keith Rollin
> On Jun 16, 2019, at 11:14, Darin Adler  wrote:
> 
> If we want to augment it, we should think of what we are aiming at. I do find 
> it useful to see which tests are failing, and when I click on the red bubble 
> I don’t see that information. I have to click once to see the “log of 
> activities” then click on “results”, then see a confusing giant file with 
> lots of other information. At the bottom of that file the one thing I want to 
> know.

We might want to also start turning those failure into action items. We could 
have an automatic mechanism that gathers the failures, records them in a 
database, and then — with sufficient data — makes determinations about the 
flakiness or other status of the test. It could then mark the test as flaky or 
raise it as an issue to some responsible (and responsive) party.

We could also have a relatively manual process. The failures are surfaced in 
Bugzilla or in a Bugzilla-accessible page. The engineer posting the patch could 
then review the failures and mark them as “Flag as flaky”, “Flag as failing and 
should be fixed by someone else”, “Flag as failing and should be ignored”, etc. 
These responses could then be turned into action items for some responsible 
(and responsive) party to address.

As Michael says, there’s a big issue with ignoring test results. Putting a 
frictionless process in place to address test results would help make them more 
effective. When I make a change to an Xcode project and Windows builds throw up 
errors, that’s not something caused by my immediate patch, but I would like to 
see the flaky test fixed.

— Keith

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


Re: [webkit-dev] Reminder regarding formatting uint64_t

2019-02-27 Thread Keith Rollin
> On Wed, Feb 27, 2019 at 2:36 PM Michael Catanzaro  
> wrote:
> Hi,
> 
> For the past several years, I've been regularly fixing -Wformat 
> warnings that look like this:
> 
> ../../Source/WebKit/WebProcess/WebPage/WebPage.cpp:3148:46: warning: 
> format ‘%llu’ expects argument of type ‘long long unsigned 
> int’, but argument 6 has type ‘uint64_t’ {aka ‘long unsigned 
> int’} [-Wformat=]
>  RELEASE_LOG_ERROR(ProcessSuspension, "%p - WebPage 
> (PageID=%llu) - LayerTreeFreezeReason::ProcessSuspended was set when 
> removing LayerTreeFreezeReason::PageTransition; current reasons are %d",
> 
> ^~
>  this, m_pageID, m_LayerTreeFreezeReasons.toRaw());
>
> 
> Problem is that uint64_t is long long unsigned int on Mac, but only 
> long unsigned int on Linux. It can't be printed with %llu, so please 
> use PRIu64 instead. E.g.:
> 
> LOG("%llu", pageID); // wrong
> LOG("%" PRIu64, pageID); // correct
> 
> This is good to keep in mind for any sized integer types, but uint64_t 
> in particular since this is what causes problems for us in practice.



> On Feb 27, 2019, at 14:47, Ryosuke Niwa  wrote:
> 
> We should probably stop using these formatting strings in favor of makeString 
> by making *LOG* functions to use makeString behind the scene.

Formatting strings are pretty much required for the RELEASE_LOG* macros. These 
macros require a string literal for the formatting information, so you can’t 
say something like:

RELEASE_LOG_ERROR(ProcessSuspension, makeString(this, " - WebPage (PageID = ", 
m_pageID, "), - LayerTreeFreezeReason::ProcessSuspended was set when removing 
LayerTreeFreezeReason::PageTransition; current reasons are ", 
m_LayerTreeFreezeReasons.toRaw()));

This would not result in a string literal being passed to RELEASE_LOG, and 
you’d get a compiler error.

You could technically get away with passing your created string along with a 
“%s” format specification:

RELEASE_LOG_ERROR(ProcessSuspension, "%s", makeString(this, " - WebPage (PageID 
= ", m_pageID, "), - LayerTreeFreezeReason::ProcessSuspended was set when 
removing LayerTreeFreezeReason::PageTransition; current reasons are ", 
m_LayerTreeFreezeReasons.toRaw()));

But this is not advised. The underlying Cocoa os_log facility is able to 
greatly compress the logs stored in memory and on disk, as well as get 
corresponding performance benefits, by taking advantage of the fact that the 
formatting string is a literal that can be found in the executable’s binary 
image. When you log with a particular formatting string, that string is not 
included in the log archive, but a reference to it is. In the case that Michael 
gives, a reference to the big formatting string is stored along with the 
pointer, the unsigned long long, and the integer. In the above example, a 
reference to “%s” is stored, along with the fully-formatted string. This latter 
approach takes up a lot more memory.

So go wild with the LOG* macros, but understand the restrictions with the 
RELEASE_LOG* macros.

— Keith

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


Re: [webkit-dev] Unified sources have broken our #include hygiene

2018-09-04 Thread Keith Rollin
> On Sep 1, 2018, at 19:25, Darin Adler  wrote:
> 
> I’ve long been interested in  and wondered 
> whether we could use it, but it sort of does the opposite, and the “multiple 
> platforms” problem could well make even that tool impractical for us.

I’ve spent a couple of weeks-of-code looking into IWYU. It didn't seem 
practical for us. Our source code is not quite in the shape that IWYU desires 
and getting it into that shape would take a lot of effort (your comment about 
multiple platforms being an example of the issues I faced). And a trial of just 
running WTF through IWYU didn’t show any progress towards my goal at the time, 
which was to speed up builds by reducing the number of #includes. Keith 
Miller’s approach with the Unified Sources was way more effective. Establishing 
a goal of using IWYU to solve Simon’s problem is probably more trouble than 
it’s worth, IMO.

— Keith

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


Re: [webkit-dev] Compile time increase over time

2017-04-24 Thread Keith Rollin
> On Apr 24, 2017, at 11:15, JF Bastien  wrote:
> 
> 1) Reduce #includes by doing more forward declaring and less inlining.  We 
> would probably need link time optimization to not lose performance benefits 
> of inlining functions in headers.
> 
> https://include-what-you-use.org/
> ?

I did some work on this last year. I didn’t get the work done and don’t have 
any preliminary results to report, but it’s archived away for when I can get 
back to it or if anyone else wants to look into it.

— Keith

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