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