Re: [webkit-dev] Announcing EWS status-bubbles on Github PRs

2022-08-17 Thread Saam Barati via webkit-dev
Thanks, this is a big improvement over what's there now.

Question below:

> On Aug 17, 2022, at 2:03 PM, Aakash Jain via webkit-dev 
>  wrote:
> 
> Hi Everyone,
> 
> I am happy to announce EWS status-bubbles on Github PRs. We’ve ported many of 
> the best features of Bugzilla EWS status-bubbles to a GitHub compatible 
> format.
> 
> From now on, when you create any new GitHub PR, you will automatically see 
> EWS status-bubbles on your PRs. We also mirror the latest status-bubble on 
> top (in PR description) so that it is always easy to find consistently in 
> same place. Due to different architecture, we wouldn't be porting the 
> "position in queue" feature from Bugzilla status-bubbles to GitHub 
> status-bubbles.

Why did you choose not to port "position in queue" this over to GitHub? I find 
this genuinely useful and I think it's a usability regression to lose it.

- Saam

> 
> 
> Following are the main benefits of status-bubbles over GitHub checks:
> - status-bubbles are displayed for all the queues, even if builds haven't 
> started on those queues
> - in case PR has multiple iterations, status-bubbles are displayed for all 
> the iterations
> - displayed even after a PR is merged/closed
> - easy to glance through the status of all the queues
> 
> 
> Upcoming features:
> - Retry failed build support https://webkit.org/b/244055
> - Generate detailed hover-over text for each status-bubble (including 
> information about build-steps) https://webkit.org/b/244053
> - Orange status-bubble equivalent in Github status-bubbles 
> https://webkit.org/b/244054
> 
> If you notice any issue, please feel free to file bugs (and assign to me).
> 
> Thanks
> Aakash
> ___
> 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] Elliott Williams is now a reviewer

2022-08-05 Thread Saam Barati via webkit-dev
Hi floks,

Elliott Williams is now a WebKit reviewer 🥳

Elliott is an expert in WebKit’s build system. Please reach out to them for 
reviews on your patches.

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


Re: [webkit-dev] ChangeLog Deprecation Plans

2022-04-18 Thread Saam Barati via webkit-dev
I think we need both of these things, and I don't think they should be mutually 
exclusive. Having motivation/design/details/gotchas go into a "changelog" 
accompanying a commit is very useful when blaming code.

- Saam

> On Apr 18, 2022, at 6:49 PM, Fujii Hironori via webkit-dev 
>  wrote:
> 
> 
> On Tue, Apr 19, 2022 at 6:55 AM Yusuke Suzuki via webkit-dev 
> mailto:webkit-dev@lists.webkit.org>> wrote:
> I think this is important. We are using commit message / ChangeLog as a 
> document tied to the change, and we are writing very detailed description to 
> make the intention / design of the change clear and making it as a good 
> document when we read the change via git-blame, bisect, using that header, 
> investigating how it works etc.
> To make / keep this commit message / ChangeLog helpful, we need review, and I 
> think reviewing of these messages is critical to keep usefulness of them.
> 
> 
> I don't think commit messages and ChangeLogs are the best place for technical 
> descriptions.
> We use them because we don't have a better place.

> 
> libpas added the technical document in the repository in markdown.
> https://github.com/WebKit/WebKit/blob/main/Source/bmalloc/libpas/Documentation.md
>  
> 
> This makes it possible to change code and update the document in a single 
> commit, and get reviewed.
> markdown is better than plain text. Updated documents are more useful than 
> the historical wiki pages.
> It'd be nice if more documents are migrated into the repository.
>  
> ___
> 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] Request for Position on Exception Handling in WebAssembly

2021-08-30 Thread Saam Barati via webkit-dev
Hi Andreas,

We're in favor of this proposal.

Tracking work for this proposal in: 
https://bugs.webkit.org/show_bug.cgi?id=229681

Cheers,

- Saam

> On Aug 27, 2021, at 3:26 AM, Andreas Haas via webkit-dev 
>  wrote:
> 
> Hello webkit-dev,
> 
> We would like to get an official position from WebKit on the Exception 
> Handling in WebAssembly proposal 
> (https://github.com/WebAssembly/exception-handling 
> ). We experimented with 
> WebAssembly Exception Handling successfully in an Origin Trial and plan to 
> ship our implementation soon.
> 
> Cheers, Andreas Haas
> ___
> 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] Caio Lima is now a WebKit reviewer

2021-03-18 Thread Saam Barati via webkit-dev
Hi folks,

I'm happy to announce that Caio Lima is now a WebKit reviewer. Send your 
JavaScriptCore reviews his way!

Congrats, Caio.

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


Re: [webkit-dev] Request for position: top-level await in JS

2020-09-24 Thread Saam Barati
We like top-level await.

- Saam

> On Sep 24, 2020, at 1:36 PM, Shu-yu Guo  wrote:
> 
> Hi all,
> 
> This is a request for the WebKit position on the top-level await JS proposal, 
> which has been at Stage 3 in TC39 since June, 2019. Chrome is planning to 
> ship it soon, likely in 88.
> 
> (Interestingly, Node has flipped on the feature flag in V8 for top-level 
> await by default, so it is already shipping for the Node ecosystem: 
> https://github.com/nodejs/node/pull/34558 
> )
> 
> - Proposal: https://github.com/tc39/proposal-top-level-await 
> 
> - Specification: https://tc39.es/proposal-top-level-await/ 
> 
> - HTML integration PR: https://github.com/whatwg/html/pull/4352 
> 
> - ChromeStatus: https://chromestatus.com/feature/5767881411264512 
> 
> 
> Cheers,
> Shu-yu Guo, V8
> 
> ___
> 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] Problems with code review tool

2020-09-22 Thread Saam Barati
Hi Ken,

I think this is a known problem. Unfortunately, this happens to me a few times 
a year as well.

- Saam

> On Sep 21, 2020, at 2:26 PM, Ken Russell  wrote:
> 
> Hi,
> 
> Often when I publish code reviews on bugs.webkit.org 
> , my browser tab gets stuck waiting for 
> ews.webkit.org . Reloading the bug shows my comments, 
> but because the submission doesn't complete, the review comments are still 
> treated as pending by the tool, so publishing again causes duplicates.
> 
> Has anyone else run into this?
> 
> (My browser is Chrome on macOS.)
> 
> Thanks,
> 
> -Ken
> 
> 
> -- 
> I support flexible work schedules, and I’m sending this email now because it 
> is within the hours I’m working today.  Please do not feel obliged to reply 
> straight away - I understand that you will reply during the hours you work, 
> which may not match mine. (credit: jparent@)
> 
> ___
> 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] Switching open source Debug bots to building and testing with configuration --force-opt=O3

2020-06-24 Thread Saam Barati
d to mention that we saw unexplained and very large impact on JSC 
>>>> test speed from enabling/disabling TCSM. That may be a good thing to look 
>>>> into while optimizing JSC test speed.
>>>> 
>>>> - Alexey
>>>> 
>>>> 
>>>>> As for non-JSC test runs, I have not actually measured what the time 
>>>>> savings are.  Given there is resistance to going with O3 there, we don’t 
>>>>> have to share the build artifacts for running the tests.  JSC test runs 
>>>>> should be able to just build JSC for each O3 Debug JSC test run and it is 
>>>>> still a win over the current status quo i.e. test runs never complete.
>>>>> 
>>>>> Regarding Geoff’s earlier question about whether I know for sure that 
>>>>> switching to O3 will fix the current Debug JSC bot failures to run tests, 
>>>>> the answer is I’m not certain.  The failure is a timeout due to the 
>>>>> master bot not seeing any output from the tester bot for more than 20 
>>>>> minutes.  I’ve not been able to reproduce this yet.  But with a Debug 
>>>>> build test run taking 4+ hours, it can only be a progression to switch 
>>>>> the Debug JSC test bots to O3.
>>>>> 
>>>>> Mark
>>>>> 
>>>>> 
>>>>>> 
>>>>>>>> And again, on the run more tests front, it would be helpful to know 
>>>>>>>> whether this change was enough to get the stress tests running or not.
>>>>>>> My experience running the tests locally supports this fully. I don't 
>>>>>>> get timeouts when running O3+Debug locally. When running Debug+O0 
>>>>>>> locally, I'd get timeouts all the time, and the total test run would 
>>>>>>> take ~4-8 hours. We can wait for official confirmation from Mark.
>>>>>> 
>>>>>> Alexey, do the JSC stress tests run now on bots? If not, how fast would 
>>>>>> they need to run in order to be eligible to run on bots?
>>>>>> 
>>>>>> Thanks,
>>>>>> Geoff
>>>>>> 
>>>>>>> 
>>>>>>> - Saam
>>>>>>> 
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Geoff
>>>>>>>> 
>>>>>>>>> On Jun 18, 2020, at 9:30 PM, Saam Barati >>>>>>>> <mailto:sbar...@apple.com>> wrote:
>>>>>>>>> 
>>>>>>>>> Why are we insisting on doing something on the bots that takes ~10x 
>>>>>>>>> longer to run than necessary? I’d rather have that time spent running 
>>>>>>>>> more tests.
>>>>>>>>> 
>>>>>>>>> Overall, how we’re doing things now feels like a bad allocation of 
>>>>>>>>> bot resources. The differences I see between O3 with no-inlining vs 
>>>>>>>>> O0 is:
>>>>>>>>> - Some race conditions will behave differently. Race conditions are 
>>>>>>>>> already non predictable. I don’t think we’re losing anything here.
>>>>>>>>> - O0 vs O3 is a different compiler. We may encounter bugs in O3 we 
>>>>>>>>> don’t in O0, and vice versa. In general, we probably care more about 
>>>>>>>>> O3 compiler bugs than O0, since we don’t ship O0, but ship a lot of 
>>>>>>>>> O3.
>>>>>>>>> 
>>>>>>>>> (And if we’re going to insist on “I want it to run what I build at my 
>>>>>>>>> desk”: I run debug with O3 at my desk, and I can run debug tests in a 
>>>>>>>>> reasonable amount of time now.)
>>>>>>>>> 
>>>>>>>>> In evaluating what’s the better setup, I think it’s helpful to think 
>>>>>>>>> about this from the other side. Let’s imagine we had Debug+O3 as our 
>>>>>>>>> current setup. And someone proposed to move it to O0 and make our 
>>>>>>>>> tests take ~10x longer. I think that’d be a non-starter.
>>>>>>>>> 
>>>>>>>>> - Saam
>>>>>>>>> 
>>>>>>>>>> On Jun 17, 2020, at 9:48 PM, Simon Fraser >>>>>>&g

Re: [webkit-dev] Switching open source Debug bots to building and testing with configuration --force-opt=O3

2020-06-19 Thread Saam Barati


> On Jun 19, 2020, at 1:04 PM, Alexey Proskuryakov  wrote:
> 
> 
> 
>> 19 июня 2020 г., в 10:24 AM, Geoffrey Garen > <mailto:gga...@apple.com>> написал(а):
>> 
>>>> On Jun 19, 2020, at 8:04 AM, Geoffrey Garen >>> <mailto:gga...@apple.com>> wrote:
>>>> 
>>>> Can you explain more about what "O3 with no-inlining” is? How does 
>>>> --force-opt=O3 avoid inlining? Would this fully resolve Simon concern 
>>>> about stack traces, or would something still be different about stack 
>>>> traces?
>>> There doesn't exist a way to do this now, but it'd be trivial to add a way. 
>>> I won't claim it fixes all stack traces differences, but I'd think 
>>> compiling using "-fno-inline -fno-optimize-sibling-calls" would get us 
>>> pretty far in crashing stack traces being similar enough.
>> 
>> Sounds good.
>> 
>> I think we should try to refine the proposal along these lines, to minimize 
>> the downsides. I won’t speak for Simon, but for me, being able to ensure a 
>> clear backtrace from a bot would be a big improvement.
> 
> Enabling some level of optimization is reasonable; whether it should be -O3 
> with inlining disabled or -Og is a technical question that probably can't be 
> answered without hard data. Also, building locally in the same way as bots do 
> could be a show stopper, as people don't like slow builds.
Just to be super clear, I'm not proposing turning off inlining for JSC debug 
testers. We on JSC are happy with stack traces that aren't perfect.

My no inlining is a proposal for folks if we wanted this more broadly in our 
testing. I don't know what the results of inlining are on the time Debug+O3 
takes to. run. If it's negligible, we'd also be happy to run this way for JSC 
Debug. But if JSC is the only thing making a change, we're going to run with 
inlining on.

> 
>>>> And again, I think this discussion would get a lot more focused if the 
>>>> change could apply only to JSC code, and not also to WTF code.
>>> I believe Mark's proposal, initially, is just to make JSC do this. So I 
>>> don't see the point of compiling WTF differently. JSC can kick off its own 
>>> build, and run Debug+O3 tests faster than it can run Debug+O0 tests. Given 
>>> people working on JSC want this, and people working on JSC defend these 
>>> tests, and that these test results are more stable (see below), we should 
>>> make this change for JSC.
>>> 
>>> I was trying to convince folks defending non-JSC testing, that they too, 
>>> should want this. I'm not going to pull teeth here. If folks want their 
>>> tests to take ~10x longer to run, they're entitled to make that tradeoff.
>> 
>> Got it.
> 
> To do this only for JSC builds, we'd need separate builders and storage, so 
> it becomes a question of allocating more resources, not just switching over 
> to a different configuration. While EWS builds for JSC independently, 
> post-commit testing shares build artifacts across all testers.
As Mark has shown, we can build locally and run faster than using a plain debug 
build. So as a straw man, this should just work and be faster than what we have 
today.

- Saam


> 
>>>> And again, on the run more tests front, it would be helpful to know 
>>>> whether this change was enough to get the stress tests running or not.
>>> My experience running the tests locally supports this fully. I don't get 
>>> timeouts when running O3+Debug locally. When running Debug+O0 locally, I'd 
>>> get timeouts all the time, and the total test run would take ~4-8 hours. We 
>>> can wait for official confirmation from Mark.
>> 
>> Alexey, do the JSC stress tests run now on bots? If not, how fast would they 
>> need to run in order to be eligible to run on bots?
> 
> I don't think that there is a simple answer, as certain variations of stress 
> tests get disabled on certain bots, JSC tests have a lot of variations that 
> are handpicked. I wouldn't even know how to find the complex answer, but 
> perhaps you can get the answer from <https://build.webkit.org/dashboard/ 
> <https://build.webkit.org/dashboard/>>
> 
> - Alexey
> 
> 
> 
>> Thanks,
>> Geoff
>> 
>>> 
>>> - Saam
>>> 
>>>> 
>>>> Thanks,
>>>> Geoff
>>>> 
>>>>> On Jun 18, 2020, at 9:30 PM, Saam Barati >>>> <mailto:sbar...@apple.com>> wrote:
>>>>> 
>>&

Re: [webkit-dev] Switching open source Debug bots to building and testing with configuration --force-opt=O3

2020-06-19 Thread Saam Barati


> On Jun 19, 2020, at 10:02 AM, Saam Barati  wrote:
> 
> 
> 
>> On Jun 19, 2020, at 8:04 AM, Geoffrey Garen > <mailto:gga...@apple.com>> wrote:
>> 
>> Can you explain more about what "O3 with no-inlining” is? How does 
>> --force-opt=O3 avoid inlining? Would this fully resolve Simon concern about 
>> stack traces, or would something still be different about stack traces?
> There doesn't exist a way to do this now, but it'd be trivial to add a way. I 
> won't claim it fixes all stack traces differences, but I'd think compiling 
> using "-fno-inline -fno-optimize-sibling-calls" would get us pretty far in 
> crashing stack traces being similar enough.
> 
>> 
>> And again, I think this discussion would get a lot more focused if the 
>> change could apply only to JSC code, and not also to WTF code.
> I believe Mark's proposal, initially, is just to make JSC do this. So I don't 
> see the point of compiling WTF differently. JSC can kick off its own build, 
> and run Debug+O3 tests faster than it can run Debug+O0 tests. Given people 
> working on JSC want this, and people working on JSC defend these tests, and 
> that these test results are more stable (see below), we should make this 
> change for JSC.
> 
> I was trying to convince folks defending non-JSC testing, that they too, 
> should want this. I'm not going to pull teeth here. If folks want their tests 
> to take ~10x longer to run, they're entitled to make that tradeoff.
According to Alexey, this is ~2x longer. So please replace my use of 10x here 
with 2x.

- Saam

> 
>> 
>> And again, on the run more tests front, it would be helpful to know whether 
>> this change was enough to get the stress tests running or not.
> My experience running the tests locally supports this fully. I don't get 
> timeouts when running O3+Debug locally. When running Debug+O0 locally, I'd 
> get timeouts all the time, and the total test run would take ~4-8 hours. We 
> can wait for official confirmation from Mark.
> 
> - Saam
> 
>> 
>> Thanks,
>> Geoff
>> 
>>> On Jun 18, 2020, at 9:30 PM, Saam Barati >> <mailto:sbar...@apple.com>> wrote:
>>> 
>>> Why are we insisting on doing something on the bots that takes ~10x longer 
>>> to run than necessary? I’d rather have that time spent running more tests.
>>> 
>>> Overall, how we’re doing things now feels like a bad allocation of bot 
>>> resources. The differences I see between O3 with no-inlining vs O0 is:
>>> - Some race conditions will behave differently. Race conditions are already 
>>> non predictable. I don’t think we’re losing anything here.
>>> - O0 vs O3 is a different compiler. We may encounter bugs in O3 we don’t in 
>>> O0, and vice versa. In general, we probably care more about O3 compiler 
>>> bugs than O0, since we don’t ship O0, but ship a lot of O3.
>>> 
>>> (And if we’re going to insist on “I want it to run what I build at my 
>>> desk”: I run debug with O3 at my desk, and I can run debug tests in a 
>>> reasonable amount of time now.)
>>> 
>>> In evaluating what’s the better setup, I think it’s helpful to think about 
>>> this from the other side. Let’s imagine we had Debug+O3 as our current 
>>> setup. And someone proposed to move it to O0 and make our tests take ~10x 
>>> longer. I think that’d be a non-starter.
>>> 
>>> - Saam
>>> 
>>>> On Jun 17, 2020, at 9:48 PM, Simon Fraser >>> <mailto:simon.fra...@apple.com>> wrote:
>>>> 
>>>> I also object to losing good stack traces for crashes on Debug bots.
>>>> 
>>>> Also, I don't think Debug bots should build something different from what 
>>>> I build at my desk.
>>>> 
>>>> Simon
>>>> 
>>>>> On Jun 17, 2020, at 1:36 PM, Mark Lam >>>> <mailto:mark@apple.com>> wrote:
>>>>> 
>>>>> Hi folks,
>>>>> 
>>>>> We're planning to switch the JSC EWS bot and build.webkit.org 
>>>>> <http://build.webkit.org/> Debug build and test bots to building with the 
>>>>> following set first:
>>>>> ./Tools/Scripts/set-webkit-configuration --force-opt=O3
>>>>> 
>>>>> This means the Debug builds will be built with optimization level forced 
>>>>> to O3.
>>>>> 
>>>>> Why are we doing this?
>>>>> 1. So that the JSC EWS will start catching ASSERT fa

Re: [webkit-dev] Switching open source Debug bots to building and testing with configuration --force-opt=O3

2020-06-19 Thread Saam Barati


> On Jun 19, 2020, at 8:04 AM, Geoffrey Garen  wrote:
> 
> Can you explain more about what "O3 with no-inlining” is? How does 
> --force-opt=O3 avoid inlining? Would this fully resolve Simon concern about 
> stack traces, or would something still be different about stack traces?
There doesn't exist a way to do this now, but it'd be trivial to add a way. I 
won't claim it fixes all stack traces differences, but I'd think compiling 
using "-fno-inline -fno-optimize-sibling-calls" would get us pretty far in 
crashing stack traces being similar enough.

> 
> And again, I think this discussion would get a lot more focused if the change 
> could apply only to JSC code, and not also to WTF code.
I believe Mark's proposal, initially, is just to make JSC do this. So I don't 
see the point of compiling WTF differently. JSC can kick off its own build, and 
run Debug+O3 tests faster than it can run Debug+O0 tests. Given people working 
on JSC want this, and people working on JSC defend these tests, and that these 
test results are more stable (see below), we should make this change for JSC.

I was trying to convince folks defending non-JSC testing, that they too, should 
want this. I'm not going to pull teeth here. If folks want their tests to take 
~10x longer to run, they're entitled to make that tradeoff.

> 
> And again, on the run more tests front, it would be helpful to know whether 
> this change was enough to get the stress tests running or not.
My experience running the tests locally supports this fully. I don't get 
timeouts when running O3+Debug locally. When running Debug+O0 locally, I'd get 
timeouts all the time, and the total test run would take ~4-8 hours. We can 
wait for official confirmation from Mark.

- Saam

> 
> Thanks,
> Geoff
> 
>> On Jun 18, 2020, at 9:30 PM, Saam Barati > <mailto:sbar...@apple.com>> wrote:
>> 
>> Why are we insisting on doing something on the bots that takes ~10x longer 
>> to run than necessary? I’d rather have that time spent running more tests.
>> 
>> Overall, how we’re doing things now feels like a bad allocation of bot 
>> resources. The differences I see between O3 with no-inlining vs O0 is:
>> - Some race conditions will behave differently. Race conditions are already 
>> non predictable. I don’t think we’re losing anything here.
>> - O0 vs O3 is a different compiler. We may encounter bugs in O3 we don’t in 
>> O0, and vice versa. In general, we probably care more about O3 compiler bugs 
>> than O0, since we don’t ship O0, but ship a lot of O3.
>> 
>> (And if we’re going to insist on “I want it to run what I build at my desk”: 
>> I run debug with O3 at my desk, and I can run debug tests in a reasonable 
>> amount of time now.)
>> 
>> In evaluating what’s the better setup, I think it’s helpful to think about 
>> this from the other side. Let’s imagine we had Debug+O3 as our current 
>> setup. And someone proposed to move it to O0 and make our tests take ~10x 
>> longer. I think that’d be a non-starter.
>> 
>> - Saam
>> 
>>> On Jun 17, 2020, at 9:48 PM, Simon Fraser >> <mailto:simon.fra...@apple.com>> wrote:
>>> 
>>> I also object to losing good stack traces for crashes on Debug bots.
>>> 
>>> Also, I don't think Debug bots should build something different from what I 
>>> build at my desk.
>>> 
>>> Simon
>>> 
>>>> On Jun 17, 2020, at 1:36 PM, Mark Lam >>> <mailto:mark@apple.com>> wrote:
>>>> 
>>>> Hi folks,
>>>> 
>>>> We're planning to switch the JSC EWS bot and build.webkit.org 
>>>> <http://build.webkit.org/> Debug build and test bots to building with the 
>>>> following set first:
>>>> ./Tools/Scripts/set-webkit-configuration --force-opt=O3
>>>> 
>>>> This means the Debug builds will be built with optimization level forced 
>>>> to O3.
>>>> 
>>>> Why are we doing this?
>>>> 1. So that the JSC EWS will start catching ASSERT failures.
>>>> 2. JSC stress test Debug bots have been timing out and not running tests 
>>>> at all.  Hopefully, this change will fix this issue.
>>>> 3. Tests will run to completion faster and we’ll catch regressions sooner.
>>>> 
>>>> The downside: crash stack traces will be like Release build stack traces.  
>>>> But I don’t think we should let this deter us.  It’s not like there’s no 
>>>> stack information.  And just as we do with debugging Release build test 
>>>> failures, we can alw

Re: [webkit-dev] Switching open source Debug bots to building and testing with configuration --force-opt=O3

2020-06-18 Thread Saam Barati
Why are we insisting on doing something on the bots that takes ~10x longer to 
run than necessary? I’d rather have that time spent running more tests.

Overall, how we’re doing things now feels like a bad allocation of bot 
resources. The differences I see between O3 with no-inlining vs O0 is:
- Some race conditions will behave differently. Race conditions are already non 
predictable. I don’t think we’re losing anything here.
- O0 vs O3 is a different compiler. We may encounter bugs in O3 we don’t in O0, 
and vice versa. In general, we probably care more about O3 compiler bugs than 
O0, since we don’t ship O0, but ship a lot of O3.

(And if we’re going to insist on “I want it to run what I build at my desk”: I 
run debug with O3 at my desk, and I can run debug tests in a reasonable amount 
of time now.)

In evaluating what’s the better setup, I think it’s helpful to think about this 
from the other side. Let’s imagine we had Debug+O3 as our current setup. And 
someone proposed to move it to O0 and make our tests take ~10x longer. I think 
that’d be a non-starter.

- Saam

> On Jun 17, 2020, at 9:48 PM, Simon Fraser  wrote:
> 
> I also object to losing good stack traces for crashes on Debug bots.
> 
> Also, I don't think Debug bots should build something different from what I 
> build at my desk.
> 
> Simon
> 
>> On Jun 17, 2020, at 1:36 PM, Mark Lam  wrote:
>> 
>> Hi folks,
>> 
>> We're planning to switch the JSC EWS bot and build.webkit.org Debug build 
>> and test bots to building with the following set first:
>> ./Tools/Scripts/set-webkit-configuration --force-opt=O3
>> 
>> This means the Debug builds will be built with optimization level forced to 
>> O3.
>> 
>> Why are we doing this?
>> 1. So that the JSC EWS will start catching ASSERT failures.
>> 2. JSC stress test Debug bots have been timing out and not running tests at 
>> all.  Hopefully, this change will fix this issue.
>> 3. Tests will run to completion faster and we’ll catch regressions sooner.
>> 
>> The downside: crash stack traces will be like Release build stack traces.  
>> But I don’t think we should let this deter us.  It’s not like there’s no 
>> stack information.  And just as we do with debugging Release build test 
>> failures, we can always do a Debug build locally to do our debugging.
>> 
>> We would like to apply this change to all Debug build and test bots, not 
>> just the JSC ones.  Does anyone strongly object to this change?
>> 
>> Thanks.
>> 
>> Cheers,
>> Mark
>> 
>> ___
>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] [Styling] Space between [] and () in C++ lambdas

2019-11-01 Thread Saam Barati



> On Nov 1, 2019, at 11:19 AM, Ryosuke Niwa  wrote:
> 
> Hi all,
> 
> There seems to be inconsistency in our coding style with regards to spacing 
> in lambdas.
I've noticed this too and the lack of consistency slightly annoys me.

> 
> Namely, some people write a lambda as:
> auto x = [] () { }
> 
> with a space between [] and () while others would write it as:
> 
> auto x = []() { }
> 
> without a space between the two. I'd like to require either style in our 
> guideline so that things are consistent in our codebase. Which one should we 
> use?
> 
> FWIW, I mildly prefer having a space between [] and ().
Same. My vote is to have a space.

- Saam

> 
> - R. Niwa
> ___
> 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] Can we remove XXX.order files in the tree

2019-10-17 Thread Saam Barati
Can we measure the performance impact of doing this?

If it's neutral, I think we should remove it.

- Saam

> On Oct 17, 2019, at 3:05 PM, Yusuke Suzuki  wrote:
> 
> Hello WebKittens!
> 
> Our XXX.order file (e.g. JavaScriptCore.order) is not updated so long. The 
> content is already stale, and I think this is no longer effective.
> Can we just remove these XXX.order files?
> 
> -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] Planned EWS improvements

2019-09-26 Thread Saam Barati
Hi Aakash,

Thanks for doing this work.

> On Sep 26, 2019, at 11:27 AM, Aakash Jain  wrote:
> 
> Hi Everyone,
> 
> I have been making number of improvements to EWS. I also have various planned 
> improvements to EWS. I wanted to reach out to you guys to see if anyone wants 
> me to prioritize any particular improvement(s). If there is any improvement 
> which you want to see and is not listed below, please feel free to let me 
> know. Also most of the queues have been transitioned from old to new EWS and 
> I am working on the remaining ones (jsc, windows and commit-queue).
> 
> Here is the list of improvements (in no particular order):
> 
> 1) Develop a webpage showing summary of EWS builds for a patch. This page 
> would provide the summary of important build-steps, high-level details about 
> the failure (e.g.: name of the tests which failed, or possibly relevant build 
> failure logs), and include link(s) to the Buildbot page(s). This page will 
> open on clicking the status-bubbles (and would be replacement of old EWS 
> status page like https://webkit-queues.webkit.org/patch/379563/win-ews 
> ). Currently clicking 
> the status-bubble opens the Buildbot build page, which contains a lot of 
> infrastructure details, and probably is information-overload for many 
> engineers, so this summary page should help with that. 
> https://webkit.org/b/197522 
> 
> 2) Redesign status-bubble tooltip to include more detailed information about 
> failures (e.g.: each test failure name along-with url to flakiness dashboard, 
> and url to complete results.html file, as suggested by David Kilzer in 
> https://lists.webkit.org/pipermail/webkit-dev/2019-September/030799.html 
> ). 
> We should also add the tooltip support for iPad/iPhone 
> https://webkit.org/b/201940 
> 
> 3) Add a way to retry a patch in EWS. This would allow engineers to confirm 
> that the failures indicated by EWS aren't flaky/incorrect. Maybe a good place 
> to add the 'retry' button would be the status-bubble's tool-tip (visible only 
> if the bubble is red) https://webkit.org/b/196599 
> This would be amazing. (My 2c: I'd vote for the 
> button being visible without tooltip.)

- Saam

> 
> 4) Parse the relevant build failure message from build logs (and display in 
> summary page) https://webkit.org/b/201941 
> 
> 5) Style failure should be displayed in-line on the review page along-with 
> the code, just like the reviewer's comments https://webkit.org/b/202252 
> 
> 
> 6) Add more test-suites to EWS (e.g.: LLDB tests, resultsdbpy tests) 
> https://webkit.org/b/189206 , 
> https://webkit.org/b/201928 
> 
> 7) Add commit-queue support for security bugs https://webkit.org/b/201939 
> 
> 
> 8) API tests should upload crashlogs https://webkit.org/b/201929 
> 
> 
> Thanks
> Aakash
> ___
> 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] 2019 WebKit Contributors Meeting - Registration is Open

2019-09-18 Thread Saam Barati
You need to first login to:
trac.webkit.org 

You'll get an email to verify your email.

After verifying, you'll be able to login on webkit.org 

(Thanks to Tadeu for figuring this out)

- Saam

> On Sep 18, 2019, at 2:14 PM, Chris Dumez  wrote:
> 
> Did not work for me either, got a page with text “None” shortly after logging 
> in.
> 
> --
>  Chris Dumez
> 
> 
> 
> 
>> On Sep 18, 2019, at 2:12 PM, > > > > wrote:
>> 
>> Seems like the login form isn't working—I see:
>> 
>>  MultipleObjectsReturned at /auth/wlogin/
>>  get() returned more than one MyUser -- it returned 2!
>> 
>> Perhaps I just need to be more patient? :)
>> 
>> Thanks,
>> Ross
>> 
>> On 9/18/19, 1:49 PM, "webkit-dev on behalf of Jonathan Davis" 
>> >  on behalf of j...@apple.com 
>> > wrote:
>> 
>>Hello WebKit Contributors,
>> 
>>You are invited to attend the annual WebKit Contributors Meeting to be 
>> held at the Hilton Santa Clara hotel on Friday, November 1st, 2019 from 8 AM 
>> to 6 PM Pacific.
>> 
>>Breakfast and sign-in will begin at 8 AM. Presentations will run between 
>> 9 AM to 6 PM. Lunch will be provided with all day coffee, tea and a 
>> mid-afternoon snack break. A reception is planned at the venue after the 
>> meeting with dinner and drinks from 6 PM to 9 PM.
>> 
>>To attend you must be an active WebKit contributor. The meeting will be 
>> free of charge, and registration is open. Register at 
>> https://webkit.org/meeting/  
>> 
>>The event format will include a mix of prepared talks around 40 minutes 
>> long with 10-15 minutes of questions, and spontaneous lightning talks about 
>> 5-10 minutes long. If you have a topic idea that you want to present, please 
>> email me directly.
>> 
>>We look forward to seeing you there!
>> 
>>Thank you,
>>Jon Davis
>>___
>>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 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] CI builds fail on all platforms with "ReferenceError: Can't find variable: WebAssembly"

2019-09-06 Thread Saam Barati
Those tests should be checking if WebAssembly is enabled at all before running 
the test. Something like:
if (this.WebAssembly) { ... put test here ... }

Feel free to post a patch and I can review it.

- Saam

> On Sep 6, 2019, at 2:59 AM, Eike Rathke  wrote:
> 
> Hi,
> 
> our CI builds keep failing on all platforms with the following messages:
> 
> Running 
> stress/web-assembly-constructors-should-not-override-global-object-property.js.default
> Running 
> stress/web-assembly-constructors-should-not-override-global-object-property.js.mini-mode
> stress/web-assembly-constructors-should-not-override-global-object-property.js.mini-mode:
>  Exception: ReferenceError: Can't find variable: WebAssembly
> stress/web-assembly-constructors-should-not-override-global-object-property.js.mini-mode:
>  global 
> c...@web-assembly-constructors-should-not-override-global-object-property.js:2:12
> stress/web-assembly-constructors-should-not-override-global-object-property.js.default:
>  Exception: ReferenceError: Can't find variable: WebAssembly
> stress/web-assembly-constructors-should-not-override-global-object-property.js.default:
>  global 
> c...@web-assembly-constructors-should-not-override-global-object-property.js:2:12
> stress/web-assembly-constructors-should-not-override-global-object-property.js.default:
>  ERROR: Unexpected exit code: 3
> FAIL: 
> stress/web-assembly-constructors-should-not-override-global-object-property.js.default
> stress/web-assembly-constructors-should-not-override-global-object-property.js.mini-mode:
>  ERROR: Unexpected exit code: 3
> FAIL: 
> stress/web-assembly-constructors-should-not-override-global-object-property.js.mini-mode
> 
> 
> The exceptions look quite similar to the output given at
> https://bugs.webkit.org/show_bug.cgi?id=192020
> 
>  Eike
> 
> -- 
> GPG key 0x6A6CD5B765632D3A - 2265 D7F3 A7B0 95CC 3918  630B 6A6C D5B7 6563 
> 2D3A
> ___
> 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] Tadeu and Robin are now WebKit reviewers

2019-06-10 Thread Saam Barati
Hi folks,

Tadeu and Robin are both now WebKit reviewers. Join me in congratulating them. 
Please ask them to review your code! They both have a focus in JSC.

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


Re: [webkit-dev] CI builds failing for ppc64, ppc64le and s390x

2019-06-03 Thread Saam Barati
In JavaScriptCore, we make changes all the time that could break builds along 
these lines. For this, it sounds like someone is doing a bad bitwise_cast, and 
it should be trivial to fix. My guess based on the variable names is it’s one 
of Keith’s RegExp changes.

- Saam

> On Jun 3, 2019, at 7:58 AM, Eike Rathke  wrote:
> 
> Hi,
> 
> Currently the CI builds for ppc64, pc64le and s390x are failing with the
> attached error message, taken from ppc64 but the same for all three
> platforms. It started on May-22 with the indicated changes. I assume
> there's some alignment/packing difference for these platforms, didn't
> dig into it yet; does that ring a bell with someone?
> 
> Eike
> 
> -- 
> GPG key 0x6A6CD5B765632D3A - 2265 D7F3 A7B0 95CC 3918  630B 6A6C D5B7 6563 
> 2D3A
> 
> ___
> 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] Code Style: Opinion on returning void

2019-02-20 Thread Saam Barati
I prefer it as well.

- Saam

> On Feb 20, 2019, at 6:58 AM, Chris Dumez  wrote:
> 
> I also prefer allowed returning void. 
> 
> Chris Dumez
> 
>> On Feb 19, 2019, at 10:35 PM, Daniel Bates  wrote:
>> 
>> 
>> 
>>> On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa  wrote:
>>> 
 On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates  wrote:
>>> 
 > On Feb 7, 2019, at 12:47 PM, Daniel Bates  wrote:
 >
 > Hi all,
 >
 > Something bothers me about code like:
 >
 > void f();
 > void g()
 > {
 > if (...)
 > return f();
 > return f();
 > }
 >
 > I prefer:
 >
 > void g()
 > {
 > if (...) {
 > f();
 > return
 > }
 > f();
 > }
 >
 Based on the responses it seems there is sufficient leaning to codify
 the latter style.
>>> 
>>> I don't think there is a sufficient consensus as far as I can tell. Geoff
>> 
>> I didn't get this from Geoff's remark. Geoff wrote:
>> 
>> ***“return f()” when f returns void is a bit mind bending.***
>> Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: 
>> for the former style, for the latter style, no strong opinion.
>> 
>>> and Alex both expressed preferences for being able to return void,
>> 
>> I got this from Alex's message
>> 
>>> and Saam pointed out that there is a lot of existing code which does this.
>> 
>> I did not get this. He wrote emphasis mine:
>> 
>> I've definitely done this in JSC. ***I don't think it's super common***, but 
>> I've also seen code in JSC not written by me that also does this.
>> 
>>> Zalan also said he does this in his layout code.
>> 
>> I did not get this, quoting, emphasis mine:
>> 
>> I use this idiom too in the layout code. I guess I just prefer a more
>> compact code.
>> ***(I don't feel too strongly about it though)***
>> 
>> By the way, you even acknowledged that "WebKit ... tend[s] to have a 
>> separate return.". So, I inferred you were okay with it. But from this email 
>> I am no longer sure what your position is. Please state it clearly.
>> 
>>> - R. Niwa
>>> 
>> ___
>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Exciting JS features (class fields) in need of review :)

2019-02-14 Thread Saam Barati
Hi Caitlin and Xan,

> On Feb 13, 2019, at 1:51 PM, ca...@igalia.com wrote:
> 
> Hi WebKitters,
> 
> My colleagues at Igalia have been working on a number of JS language 
> features! We want WebKit to have implementations in order to provide feedback 
> for TC39, and to help meet the requirements to have them merged into the 
> specification proper.
> 
> Unfortunately, JSC suggested reviewers have been occupied for the past 6 
> months, unable to provide feedback and help us upstream the patches. I'm 
> reaching out to see if there are other folks reading webkit-dev who might be 
> able to check on the work, see how it looks, and help us get this stuff 
> upstream. We've been doing internal reviews, but unfortunately don't yet have 
> any JSC reviewers on our team.

This is unacceptable. We shouldn’t leave a patch without feedback for so long. 
I’ll review this before EOD.

- Saam
> 
> You can find the patch for instance class fields at 
> https://bugs.webkit.org/show_bug.cgi?id=174212. Any kind of constructive 
> feedback (from anyone) would be much appreciated :)
> 
> ___
> 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] Code Style: Opinion on returning void

2019-02-07 Thread Saam Barati


> On Feb 7, 2019, at 12:53 PM, Tim Horton  wrote:
> 
> 
> 
>> On Feb 7, 2019, at 12:47 PM, Daniel Bates  wrote:
>> 
>> Hi all,
>> 
>> Something bothers me about code like:
>> 
>> void f();
>> void g()
>> {
>>if (...)
>>return f();
>>return f();
>> }
> 
> ⸘do people do this‽
I've definitely done this in JSC. I don't think it's super common, but I've 
also seen code in JSC not written by me that also does this.

- Saam

> 
>> I prefer:
>> 
>> void g()
>> {
>>if (...) {
>>f();
>>return
>>}
>>f();
>> }
>> 
>> Does it bother you? For the former? For the latter? Update our style guide?
> 
> +1 to a style guide update
> 
>> Opinions, please.
>> 
>> Dan
>> ___
>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Can we jettison Sputnik?

2019-01-15 Thread Saam barati
Hi Ross,

> On Jan 15, 2019, at 11:50 AM, ross.kirsl...@sony.com wrote:
> 
> Hi everybody,
>  
> I’d like to verify whether there’s any specific reason that the 
> LayoutTests/sputnik directory continues to exist.
>  
> According to Wikipedia, “All current Sputnik tests have been incorporated 
> into ECMA's Test262 test suite.” (Here “current” evidently means June 2011.) 
> And indeed, when making fixes to address Test262 failures in JSC, it’s quite 
> awkward to have to update the Sputnik expectations to reflect any new 
> “failures”—particularly given that it’s not sufficient to simply build JSC in 
> order to run them.
I think it’s a good idea to remove these. (An aside: in general, I’m also not a 
fan of JS tests that don’t rely on the DOM existing in LayoutTests.)

>  
> Would anyone object to a patch deleting this directory outright? The only 
> concern that occurs to me is that Test262 is not currently integrated into 
> EWS.
Maybe we should integrate these into EWS? The main downside is IIRC, these take 
a while to run. I think it’s ok to remove these even if we never add Test262 to 
EWS since we still run Test262 on bots post-commit.

- Saam

>  
> Thanks!
> Ross
> ___
> 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] Watch out for std::optional's move constructor

2018-12-14 Thread Saam Barati


> On Dec 14, 2018, at 1:59 PM, Chris Dumez  wrote:
> 
>> 
>> On Dec 14, 2018, at 1:56 PM, Saam Barati > <mailto:sbar...@apple.com>> wrote:
>> 
>> 
>> 
>>> On Dec 14, 2018, at 1:54 PM, Saam Barati >> <mailto:sbar...@apple.com>> wrote:
>>> 
>>> 
>>> 
>>>> On Dec 14, 2018, at 1:37 PM, Chris Dumez >>> <mailto:cdu...@apple.com>> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> I have now been caught twice by std::optional’s move constructor. It turns 
>>>> out that it leaves the std::optional being moved-out *engaged*, it merely 
>>>> moves its value.
>>>> 
>>>> For example, testOptional.cpp:
>>>> #include 
>>>> #include 
>>>> 
>>>> int main()
>>>> {
>>>> std::optional a = 1;
>>>> std::optional b = std::move(a);
>>>> std::cout << "a is engaged? " << !!a << std::endl;
>>>> std::cout << "b is engaged? " << !!b << std::endl;
>>>> return 0;
>>>> }
>>>> 
>>>> $ clang++ testOptional.cpp -o testOptional -std=c++17
>>>> $ ./testOptional
>>>> a is engaged? 1
>>>> b is engaged? 1
>>>> 
>>>> I would have expected:
>>>> a is engaged? 0
>>>> b is engaged? 1
>>> I would have expected this too.
>>> 
>>>> 
>>>> This impacts the standard std::optional implementation on my machine as 
>>>> well as the local copy in WebKit’s wtf/Optional.h.
>>>> 
>>>> As far as I know, our convention in WebKit so far for our types has been 
>>>> that types getting moved-out are left in a valid “empty” state.
>>> I believe it's UB to use an object after it has been moved.
>> I'm wrong here.
>> Apparently objects are left in a "valid but unspecified state".
>> 
>> https://stackoverflow.com/questions/32346143/undefined-behavior-with-stdmove 
>> <https://stackoverflow.com/questions/32346143/undefined-behavior-with-stdmove>
> I believe in WebKit, however, we’ve made sure our types are left in a valid 
> “empty” state, thus my proposal for our own optional type that would be less 
> error-prone / more convenient to use.
I think your proposal is reasonable. I agree it's less error prone.

I think if we make this change, we should pull optional out of std and put it 
in WTF, since we're now implementing behavior we will rely on being specified.

- Saam 

> 
>> 
>> - Saam
>>> 
>>> - Saam
>>> 
>>>> As such, I find that std::optional’s move constructor behavior is 
>>>> error-prone.
>>>> 
>>>> I’d like to know how do other feel about this behavior? If enough people 
>>>> agree this is error-prone, would we consider having our
>>>> own optional type in WTF which resets the engaged flag (and never allow 
>>>> the std::optional)?
>>>> 
>>>> Thanks,
>>>> --
>>>>  Chris Dumez
>>>> 
>>>> 
>>>> 
>>>> 
>>>> ___
>>>> webkit-dev mailing list
>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>> <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] Watch out for std::optional's move constructor

2018-12-14 Thread Saam Barati


> On Dec 14, 2018, at 1:54 PM, Saam Barati  wrote:
> 
> 
> 
>> On Dec 14, 2018, at 1:37 PM, Chris Dumez > <mailto:cdu...@apple.com>> wrote:
>> 
>> Hi,
>> 
>> I have now been caught twice by std::optional’s move constructor. It turns 
>> out that it leaves the std::optional being moved-out *engaged*, it merely 
>> moves its value.
>> 
>> For example, testOptional.cpp:
>> #include 
>> #include 
>> 
>> int main()
>> {
>> std::optional a = 1;
>> std::optional b = std::move(a);
>> std::cout << "a is engaged? " << !!a << std::endl;
>> std::cout << "b is engaged? " << !!b << std::endl;
>> return 0;
>> }
>> 
>> $ clang++ testOptional.cpp -o testOptional -std=c++17
>> $ ./testOptional
>> a is engaged? 1
>> b is engaged? 1
>> 
>> I would have expected:
>> a is engaged? 0
>> b is engaged? 1
> I would have expected this too.
> 
>> 
>> This impacts the standard std::optional implementation on my machine as well 
>> as the local copy in WebKit’s wtf/Optional.h.
>> 
>> As far as I know, our convention in WebKit so far for our types has been 
>> that types getting moved-out are left in a valid “empty” state.
> I believe it's UB to use an object after it has been moved.
I'm wrong here.
Apparently objects are left in a "valid but unspecified state".

https://stackoverflow.com/questions/32346143/undefined-behavior-with-stdmove 
<https://stackoverflow.com/questions/32346143/undefined-behavior-with-stdmove>

- Saam
> 
> - Saam
> 
>> As such, I find that std::optional’s move constructor behavior is 
>> error-prone.
>> 
>> I’d like to know how do other feel about this behavior? If enough people 
>> agree this is error-prone, would we consider having our
>> own optional type in WTF which resets the engaged flag (and never allow the 
>> std::optional)?
>> 
>> Thanks,
>> --
>>  Chris Dumez
>> 
>> 
>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> <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] Watch out for std::optional's move constructor

2018-12-14 Thread Saam Barati


> On Dec 14, 2018, at 1:37 PM, Chris Dumez  wrote:
> 
> Hi,
> 
> I have now been caught twice by std::optional’s move constructor. It turns 
> out that it leaves the std::optional being moved-out *engaged*, it merely 
> moves its value.
> 
> For example, testOptional.cpp:
> #include 
> #include 
> 
> int main()
> {
> std::optional a = 1;
> std::optional b = std::move(a);
> std::cout << "a is engaged? " << !!a << std::endl;
> std::cout << "b is engaged? " << !!b << std::endl;
> return 0;
> }
> 
> $ clang++ testOptional.cpp -o testOptional -std=c++17
> $ ./testOptional
> a is engaged? 1
> b is engaged? 1
> 
> I would have expected:
> a is engaged? 0
> b is engaged? 1
I would have expected this too.

> 
> This impacts the standard std::optional implementation on my machine as well 
> as the local copy in WebKit’s wtf/Optional.h.
> 
> As far as I know, our convention in WebKit so far for our types has been that 
> types getting moved-out are left in a valid “empty” state.
I believe it's UB to use an object after it has been moved.

- Saam

> As such, I find that std::optional’s move constructor behavior is error-prone.
> 
> I’d like to know how do other feel about this behavior? If enough people 
> agree this is error-prone, would we consider having our
> own optional type in WTF which resets the engaged flag (and never allow the 
> std::optional)?
> 
> Thanks,
> --
>  Chris Dumez
> 
> 
> 
> 
> ___
> 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] BigIntBench

2018-10-07 Thread Saam Barati
Hi Caio,

> On Oct 6, 2018, at 7:30 AM, Caio Lima  wrote:
> 
> Hi all.
> 
> I'm starting working to fix JIT support of BigInt in some operations
> we already have upstream. In such case, I'm sending
> (https://bugs.webkit.org/show_bug.cgi?id=186177) to support BigInt
> speculation into ValueAdd node. As I want to know if BigInt
> speculation represents some performance improvement, I'm also
> proposing a new benchmark suite called BigIntBench. The idea right now
> is to enable us to write microbenchmarks while working into JIT fixes.
> The main reason behind that is due the practicality of enabling
> "--useBigInt=true" flags for tests in this suite. The big plan is to
> move all microbenchmarks to "JSTests/microbenchmarks" and then add
> relevant tests to evaluate how fast JSC can manipulate BigInts. IIRC,
> Robin Morisset mentioned about introducing some benchmarks to evaluate
> BigInt implementation sometime ago.
> 
> What do you think about that? Does it make sense?
I think it makes a lot of sense to start using benchmarks to drive performance 
improvements in our BigInt implementation.

It's probably good to use both microbenchmarks and macro benchmarks to do this. 
I definitely agree it's worth writing microbenchmarks to show that changes are 
good for perf. But it's probably worth converting some preexisting benchmarks 
to use BigInts and measure performance on that as well. Some ideas could be 
taking some of the math-heavy benchmarks from JetStream/Kraken as a starting 
point.

- Saam

> 
> Regards,
> Caio.
> ___
> 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] [jsc-dev] Proposal: Using LLInt Asm in major architectures even if JIT is disabled

2018-09-19 Thread Saam Barati
Interesting! I must have not run this experiment correctly when I did it.

- Saam

> On Sep 19, 2018, at 7:31 PM, Yusuke Suzuki  wrote:
> 
>> On Thu, Sep 20, 2018 at 12:54 AM Saam Barati  wrote:
>> To elaborate: I ran this same experiment before. And I forgot to turn off 
>> the RegExp JIT and got results similar to what you got. Once I turned off 
>> the RegExp JIT, I saw no perf difference.
> 
> Yeah, I disabled JIT and RegExpJIT explicitly by using
> 
> export JSC_useJIT=false
> export JSC_useRegExpJIT=false
> 
> and I checked no JIT code is generated by running dumpDisassembly. And I also 
> put `CRASH()` in ExecutableAllocator::singleton() to ensure no executable 
> memory is allocated.
> The result is the same. I think `useJIT=false` disables RegExp JIT too.
> 
>baseline  patched  
> 
> 
> ai-astar  3499.046+-14.772 ^
> 1897.624+-234.517   ^ definitely 1.8439x faster
> audio-beat-detection  1803.466+-491.965  
> 970.636+-428.051 might be 1.8580x faster
> audio-dft 1756.985+-68.710 ^ 
> 954.312+-528.406   ^ definitely 1.8411x faster
> audio-fft 1637.969+-458.129  
> 850.083+-449.228 might be 1.9268x faster
> audio-oscillator  1866.006+-569.581^ 
> 967.194+-82.521^ definitely 1.9293x faster
> imaging-darkroom  2156.526+-591.042^
> 1231.318+-187.297   ^ definitely 1.7514x faster
> imaging-desaturate3059.335+-284.740^
> 1754.128+-339.941   ^ definitely 1.7441x faster
> imaging-gaussian-blur16034.828+-1930.938   ^
> 7389.919+-2228.020  ^ definitely 2.1698x faster
> json-parse-financial60.273+-4.143 
> 53.935+-28.957  might be 1.1175x faster
> json-stringify-tinderbox39.497+-3.915 
> 38.146+-9.652   might be 1.0354x faster
> stanford-crypto-aes873.623+-208.225^ 
> 486.350+-132.379   ^ definitely 1.7963x faster
> stanford-crypto-ccm538.707+-33.979 ^ 
> 285.944+-41.570^ definitely 1.8840x faster
> stanford-crypto-pbkdf21929.960+-649.861^
> 1044.320+-1.182 ^ definitely 1.8481x faster
> stanford-crypto-sha256-iterative   614.344+-200.228  
> 342.574+-123.524 might be 1.7933x faster
> 
>   2562.183+-207.456^
> 1304.749+-312.963   ^ definitely 1.9637x faster
> 
> I think this result is not related to RegExp JIT since ai-astar is not using 
> RegExp.
> 
> Best regards,
> Yusuke Suzuki
>  
>> 
>> - Saam
>> 
>>> On Sep 19, 2018, at 8:53 AM, Saam Barati  wrote:
>>> 
>>> Did you turn off the RegExp JIT?
>>> 
>>> - Saam
>>> 
>>>> On Sep 18, 2018, at 11:23 PM, Yusuke Suzuki  
>>>> wrote:
>>>> 
>>>> Hi WebKittens!
>>>> 
>>>> Recently, node-jsc is announced[1]. When I read the documents of that 
>>>> project,
>>>> I found that they use LLInt ASM interpreter instead of CLoop in non-JIT 
>>>> environment.
>>>> So I had one question in my mind: How fast the LLInt ASM interpreter when 
>>>> comparing to CLoop?
>>>> 
>>>> I've set up two builds. One is CLoop build (-DENABLE_JIT=OFF) and another 
>>>> is JIT build JSC with `JSC_useJIT=false`.
>>>> And I've ran kraken benchmarks with these two builds in x64 Linux machine. 
>>>> The results are the followings.
>>>> 
>>>> Benchmark report for Kraken on sakura-trick.
>>>> 
>>>> VMs tested:
>>>> "baseline" at 
>>>> /home/yusukesuzuki/dev/WebKit/WebKitBuild/nojit/Release/bin/jsc
>>>> "patched" at 
>>>> /home/yusukesuzuki/dev/WebKit/WebKitBuild/nojit-llint/Release/bin/jsc
>>>> 
>>>> Collected 10 samples per benchmark/VM, with 10 VM invocations per 
>>>> benchmark. Emitted a call to gc() between sample
>>>> measurements. Used 1 benchmark iteration per VM invocation for warm-up. 
>>>> Used the jsc-specific preciseTime()
>>>> function to get microsecond-level timing. Reporting benchmark execution 
>>>> times with 95% confidence intervals in
>>>> milliseconds.
>>>> 
>>>>   

Re: [webkit-dev] Proposal: Using LLInt Asm in major architectures even if JIT is disabled

2018-09-19 Thread Saam Barati
Did you turn off the RegExp JIT?

- Saam

> On Sep 18, 2018, at 11:23 PM, Yusuke Suzuki  
> wrote:
> 
> Hi WebKittens!
> 
> Recently, node-jsc is announced[1]. When I read the documents of that project,
> I found that they use LLInt ASM interpreter instead of CLoop in non-JIT 
> environment.
> So I had one question in my mind: How fast the LLInt ASM interpreter when 
> comparing to CLoop?
> 
> I've set up two builds. One is CLoop build (-DENABLE_JIT=OFF) and another is 
> JIT build JSC with `JSC_useJIT=false`.
> And I've ran kraken benchmarks with these two builds in x64 Linux machine. 
> The results are the followings.
> 
> Benchmark report for Kraken on sakura-trick.
> 
> VMs tested:
> "baseline" at /home/yusukesuzuki/dev/WebKit/WebKitBuild/nojit/Release/bin/jsc
> "patched" at 
> /home/yusukesuzuki/dev/WebKit/WebKitBuild/nojit-llint/Release/bin/jsc
> 
> Collected 10 samples per benchmark/VM, with 10 VM invocations per benchmark. 
> Emitted a call to gc() between sample
> measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used 
> the jsc-specific preciseTime()
> function to get microsecond-level timing. Reporting benchmark execution times 
> with 95% confidence intervals in
> milliseconds.
> 
>baseline  patched  
> 
> 
> ai-astar  3619.974+-57.095 ^
> 2014.835+-59.016^ definitely 1.7967x faster
> audio-beat-detection  1762.085+-24.853 ^
> 1030.902+-19.743^ definitely 1.7093x faster
> audio-dft 1822.426+-28.704 ^ 
> 909.262+-16.640^ definitely 2.0043x faster
> audio-fft 1651.070+-9.994  ^ 
> 865.203+-7.912 ^ definitely 1.9083x faster
> audio-oscillator  1853.697+-26.539 ^ 
> 992.406+-12.811^ definitely 1.8679x faster
> imaging-darkroom  2118.737+-23.219 ^
> 1303.729+-8.071 ^ definitely 1.6251x faster
> imaging-desaturate3133.654+-28.545 ^
> 1759.738+-18.182^ definitely 1.7808x faster
> imaging-gaussian-blur16321.090+-154.893^
> 7228.017+-58.508^ definitely 2.2580x faster
> json-parse-financial57.256+-2.876 
> 56.101+-4.265   might be 1.0206x faster
> json-stringify-tinderbox38.470+-2.788  ?  
> 38.771+-0.935 ?
> stanford-crypto-aes851.341+-7.738  ^ 
> 485.438+-13.904^ definitely 1.7538x faster
> stanford-crypto-ccm556.133+-6.606  ^ 
> 264.161+-3.970 ^ definitely 2.1053x faster
> stanford-crypto-pbkdf21945.718+-15.968 ^
> 1075.013+-13.337^ definitely 1.8099x faster
> stanford-crypto-sha256-iterative   623.203+-7.604  ^ 
> 349.782+-12.810^ definitely 1.7817x faster
> 
>   2596.775+-14.857 ^
> 1312.383+-8.840 ^ definitely 1.9787x faster
> 
> Surprisingly, LLInt ASM interpreter is significantly faster than CLoop. I 
> expected it would be fast, but it would show around 10% performance win.
> But the reality is that it is 2x faster. It is too much number to me to 
> consider enabling LLInt ASM interpreter for non-JIT build configuration.
> As a bonus, LLInt ASM interpreter offers sampling profiler support even in 
> non-JIT environment.
> 
> So my proposal is, how about enabling LLInt ASM interpreter in non-JIT 
> configuration environment in major architectures (x64 and ARM64)?
> 
> Best regards,
> Yusuke Suzuki
> 
> [1]: https://lists.webkit.org/pipermail/webkit-dev/2018-September/030140.html
> ___
> 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] [jsc-dev] Proposal: Using LLInt Asm in major architectures even if JIT is disabled

2018-09-19 Thread Saam Barati
To elaborate: I ran this same experiment before. And I forgot to turn off the 
RegExp JIT and got results similar to what you got. Once I turned off the 
RegExp JIT, I saw no perf difference.

- Saam

> On Sep 19, 2018, at 8:53 AM, Saam Barati  wrote:
> 
> Did you turn off the RegExp JIT?
> 
> - Saam
> 
>> On Sep 18, 2018, at 11:23 PM, Yusuke Suzuki  
>> wrote:
>> 
>> Hi WebKittens!
>> 
>> Recently, node-jsc is announced[1]. When I read the documents of that 
>> project,
>> I found that they use LLInt ASM interpreter instead of CLoop in non-JIT 
>> environment.
>> So I had one question in my mind: How fast the LLInt ASM interpreter when 
>> comparing to CLoop?
>> 
>> I've set up two builds. One is CLoop build (-DENABLE_JIT=OFF) and another is 
>> JIT build JSC with `JSC_useJIT=false`.
>> And I've ran kraken benchmarks with these two builds in x64 Linux machine. 
>> The results are the followings.
>> 
>> Benchmark report for Kraken on sakura-trick.
>> 
>> VMs tested:
>> "baseline" at /home/yusukesuzuki/dev/WebKit/WebKitBuild/nojit/Release/bin/jsc
>> "patched" at 
>> /home/yusukesuzuki/dev/WebKit/WebKitBuild/nojit-llint/Release/bin/jsc
>> 
>> Collected 10 samples per benchmark/VM, with 10 VM invocations per benchmark. 
>> Emitted a call to gc() between sample
>> measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used 
>> the jsc-specific preciseTime()
>> function to get microsecond-level timing. Reporting benchmark execution 
>> times with 95% confidence intervals in
>> milliseconds.
>> 
>>baseline  patched 
>>  
>> 
>> ai-astar  3619.974+-57.095 ^
>> 2014.835+-59.016^ definitely 1.7967x faster
>> audio-beat-detection  1762.085+-24.853 ^
>> 1030.902+-19.743^ definitely 1.7093x faster
>> audio-dft 1822.426+-28.704 ^ 
>> 909.262+-16.640^ definitely 2.0043x faster
>> audio-fft 1651.070+-9.994  ^ 
>> 865.203+-7.912 ^ definitely 1.9083x faster
>> audio-oscillator  1853.697+-26.539 ^ 
>> 992.406+-12.811^ definitely 1.8679x faster
>> imaging-darkroom  2118.737+-23.219 ^
>> 1303.729+-8.071 ^ definitely 1.6251x faster
>> imaging-desaturate3133.654+-28.545 ^
>> 1759.738+-18.182^ definitely 1.7808x faster
>> imaging-gaussian-blur16321.090+-154.893^
>> 7228.017+-58.508^ definitely 2.2580x faster
>> json-parse-financial57.256+-2.876 
>> 56.101+-4.265   might be 1.0206x faster
>> json-stringify-tinderbox38.470+-2.788  ?  
>> 38.771+-0.935 ?
>> stanford-crypto-aes851.341+-7.738  ^ 
>> 485.438+-13.904^ definitely 1.7538x faster
>> stanford-crypto-ccm556.133+-6.606  ^ 
>> 264.161+-3.970 ^ definitely 2.1053x faster
>> stanford-crypto-pbkdf21945.718+-15.968 ^
>> 1075.013+-13.337^ definitely 1.8099x faster
>> stanford-crypto-sha256-iterative   623.203+-7.604  ^ 
>> 349.782+-12.810^ definitely 1.7817x faster
>> 
>>   2596.775+-14.857 ^
>> 1312.383+-8.840 ^ definitely 1.9787x faster
>> 
>> Surprisingly, LLInt ASM interpreter is significantly faster than CLoop. I 
>> expected it would be fast, but it would show around 10% performance win.
>> But the reality is that it is 2x faster. It is too much number to me to 
>> consider enabling LLInt ASM interpreter for non-JIT build configuration.
>> As a bonus, LLInt ASM interpreter offers sampling profiler support even in 
>> non-JIT environment.
>> 
>> So my proposal is, how about enabling LLInt ASM interpreter in non-JIT 
>> configuration environment in major architectures (x64 and ARM64)?
>> 
>> Best regards,
>> Yusuke Suzuki
>> 
>> [1]: https://lists.webkit.org/pipermail/webkit-dev/2018-September/030140.html
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> ___
> jsc-dev mailing list
> jsc-...@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/jsc-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] node-jsc: A node.js port to the JavaScriptCore engine and iOS

2018-09-16 Thread Saam Barati
Hi Koby,

This sounds awesome. A good list of people to CC on the bug for reviews are:

sbar...@apple.com,fpi...@apple.com,keith_mil...@apple.com,mark@apple.com,msab...@apple.com,yusukesuz...@slowstart.org

- Saam

> On Sep 16, 2018, at 11:44 AM, Filip Pizlo  wrote:
> 
> 
> 
>> On Sep 16, 2018, at 2:09 AM, Koby Boyango  wrote:
>> 
>> Thanks for taking the time to look into the project :)
>> 
>> Filip - I would love to. Should I create one bug for all of the patches, or 
>> a bug for each patch? 
>> Also, there is an existing bug that I've reported a while ago, but worked 
>> around it for now: https://bugs.webkit.org/show_bug.cgi?id=184232. It isn't 
>> relevant in newer versions of node (it came from node's Buffer constructor, 
>> which have changed since), but I'll still be happy to send a patch if needed.
> 
> I think that you want a parent bug that’s just an umbrella and then have bugs 
> that block it for each patch.
> 
> -Filip
> 
>> 
>> Yusuke - It's interesting to compare, especially on an iOS device. I will 
>> also try to do some measurements :) Do you have a benchmark you recommend?
>> But assuming it is worth it, enabling LLInt ASM without the JIT would be 
>> great as it would probably reduce the binary size and compilation time by 
>> quite a bit.  
>> NativeScript is also using it without the JIT (and they link to an article 
>> containing some benchmarks), so they would profit from this too.
>> https://github.com/NativeScript/ios-runtime/commit/1528ed50f85998147b190c22a390b5eca36c5acb
>> 
>> Koby
>> 
>>> On Sat, Sep 15, 2018 at 2:51 AM Yusuke Suzuki  
>>> wrote:
>>> Really great!
>>> 
>>> node-jsc sounds very exciting to me. From the users' view, t is nice if we 
>>> run app constructed in node.js manner in iOS devices.
>>> In addition, from the JSC developers' view, it is also awesome. It allows 
>>> us to easily run node.js libraries / benchmarks / tests on JSC, which is 
>>> really great since,
>>> 
>>> 1. We can run tests designed for node.js, it makes our JSC implementation 
>>> more solid.
>>> 2. We can run benchmarks designed for node.js including JS libraries. JS 
>>> libraries distributed in npm are more and more used in both node.js and 
>>> browser world.
>>> If we can have a way to run benchmarks in  popular libraries on JSC easily, 
>>> that offers great opportunities to optimize JSC on them.
>>> 
 On Sat, Sep 15, 2018 at 5:20 AM Filip Pizlo  wrote:
 Wow!  That’s pretty cool!
 
 I think that it would be great for this to be upstreamed. Can you create a 
 bug on bugs.webkit.org and post your patches for review?
 
 -Filip
 
> On Sep 13, 2018, at 4:02 PM, Koby Boyango  wrote:
> 
> Hi,
> 
> I'm Koby Boyango, a senior researcher and developer at mce, and I've 
> created node-jsc, an experimental port of node.js to the JavaScriptCore 
> engine and iOS specifically.
> 
> node-jsc's core component, "jscshim" (deps/jscshim), implements (parts 
> of) v8 API on top of JavaScriptCore. It contains a stripped down version 
> of WebKit's source code (mainly JSC and WTF). To build WebKit, I'm using 
> CMake to build the JSCOnly port, with JSC\WTF compiled as static 
> libraries. For iOS I'm using my own build script with a custom toolchain 
> file.
> 
>>> I'm really happy to hear that your node-jsc is using JSCOnly ports :)
> The project also includes node-native-script, NativeScript's iOS runtime 
> refactored as node-jsc native module, allowing access to native iOS APIs 
> directly from javascript.
> 
> So first of all, I wanted to share this project with the WebKit developer 
> community.
> It's my first time working with WebKit, and node-jsc has been a great 
> opportunity to experiment with it.
> 
> Second, as I needed to make some minor changes\additions, I'm using my 
> own fork. I would love to discuss some of the changes I've made, and 
> offer some patches if you'll find them useful. 
> "WebKit Fork and Compilation" describes WebKit's usage in node-jsc and 
> the major changes\additions I've made in my fork (node-jsc's README and 
> jschim's documentation contains some more information).
> 
>>> Great, it is really nice if you have a patch for upstream :)
>>> Looking through the documents, I have one question on LLInt v.s. CLoop.
>>> 
>>> https://github.com/mceSystems/node-jsc/blob/master/deps/jscshim/docs/webkit_fork_and_compilation.md#webkit-port-and-compilation
>>> > Use the optimized assembly version of LLInt (JSC's interpreter), not 
>>> > cloop. This requires enabling JIT support, although we won't be using the 
>>> > JIT (but we can omit the FTL jit).
>>> 
>>> I would like to know how fast LLInt ASM interpreter is when comparing CLoop 
>>> interpreter.
>>> If it shows nice speedup, enabling LLInt ASM interpreter without JIT for 
>>> major architectures (x64, ARM64) sounds nice.
>>> As a bonus, if we offer this build conf

Re: [webkit-dev] Can we drop supporting mixed Wasm::MemoryMode in one process?

2018-08-28 Thread Saam Barati
I would also expect bytecode interpreter compilation to be so much faster than 
JIT compilation that we could laziliy compile either:
- When we instantiate
- When functions are called for the first time

- Saam

> On Aug 28, 2018, at 12:17 PM, Yusuke Suzuki  
> wrote:
> 
> 
> 
> On Wed, Aug 29, 2018 at 4:10 Filip Pizlo  > wrote:
> 
>> On Aug 28, 2018, at 12:09 PM, Yusuke Suzuki > > wrote:
>> 
>> 
>> 
>> On Wed, Aug 29, 2018 at 3:58 Filip Pizlo > > wrote:
>> 
>>> On Aug 28, 2018, at 11:56 AM, Yusuke Suzuki >> > wrote:
>>> 
>>> 
>>> 
>>> On Wed, Aug 29, 2018 at 3:49 Yusuke Suzuki >> > wrote:
>>> 
>>> 
>>> On Wed, Aug 29, 2018 at 3:27 Filip Pizlo >> > wrote:
>>> 
 On Aug 28, 2018, at 11:25 AM, Yusuke Suzuki >>> > wrote:
 
 Thanks!
 
 On Wed, Aug 29, 2018 at 3:22 Filip Pizlo >>> > wrote:
 I don’t like this proposal.
 
 If we are running low on memory, we should switch to bounds checked memory.
 
 How about using bound checking mode exclusively for low environment?
>>> 
>>> That would mean that, paradoxically, having a machine with a lot of memory 
>>> means being able to spawn fewer wasm instances.
>>> 
>>> We want to support lightweight wasm instances because it wouldn’t be good 
>>> to rule that out as a use case.
>>> 
>>> Hmmm, can we compile the BBQ phase / initial wasm code without knowing the 
>>> attached memory’s mode? The current strategy basically defers compilation 
>>> of wasm module until the memory mode is found.
>>> Because of this, WebAssembly.compile is not so meaningful in our 
>>> implementation right now...
>>> And wasm ES6 module can import memory externally. This means that we cannot 
>>> start wasm compilation after the memory mode of the impprted memory 
>>> (described in the imported modulr) is downloaded, analyzed and found.
>>> 
>>> How about always compiling BBQ code with bound checking mode?
>>> It should work with signaling memory with small / no tweaks. And OMG code 
>>> will be compiled based on the memory mode attached to the module.
>>> Since BBQ -> OMG function call should be linked, we need to call 
>>> appropriate func for the running memory mode, but it would not introduce 
>>> significant complexity.
>> 
>> What complexity are you trying to fix, specifically?
>> 
>> What I want is starting compilation before the memory is attached a.k.a. 
>> instantiated)
>> 
>> 
>> I think that what we really want is an interpreter as our baseline.  Then 
>> tier-up to BBQ or OMG from there.  In that world, I don’t think any of this 
>> matters.
>> 
>> Does this interpreter execute wasm binary directly? If so, we can skip 
>> compiling and all should work well!
>> 
>> Even if we want some own bytecode (like stack VM to register VM etc.), it is 
>> ok if the compilation result is not tied to the memory mode.
> 
> I don’t know if it will execute the wasm binary directly, but whatever 
> bytecode it runs could be dissociated from memory mode.
> 
> Thanks, that sounds reasonable!
> If the bytecode compilation etc. is disassociated from the memory mode, the 
> bytecode can be compiled before instantiated. It means that wasm module can 
> be shared between workers (like postMessage the wasm module having compiled 
> bytecode). And we can start compiling before the memory is attached 
> (instantiated).
> 
> 
> 
> -Filip
> 
> 
>> 
>> If the compilation result is tied to the memory mode, then we still need to 
>> defer the compilation until the memory mode is attached.
>> 
>> 
>> -Filip
>> 
>> 
>>> 
>>> 
>>> 
>>> 
>>> -Filip
>>> 
>>> 
 
 
 -Filip
 
 
 
> On Aug 28, 2018, at 11:21 AM, Yusuke Suzuki  > wrote:
> 
 
 
> Posted this mail to webkit-dev mailing list too :)
> 
> On Wed, Aug 29, 2018 at 3:19 AM Yusuke Suzuki  > wrote:
> Hi JSC folks,
> 
> In Wasm supported environment, our MemoryMode is a bit dynamic.
> When we fail to allocate WasmMemory for signaling mode, we fall back to 
> the bound checking memory instead.
> 
> But Wasm code compiled for signaling / bound checking is incompatible. If 
> the code is compiled
> as signaling mode, and if we attach the memory for bound checking, we 
> need to recompile the
> code for bound checking mode. This introduces significant complexity to 
> our wasm compilation.
> And our WebAssembly.compile is not basically compiling: it is just 
> validating.
> Actual compiling needs to be deferred until the memory is attached by 
> instantiating.
> It is not good when we would like to share WasmModule among multiple wasm 
> threads / workers in the future, since the "compiled" Wasm modul

Re: [webkit-dev] [jsc-dev] Not adding DFG 32bit specialized code as much as possible!

2018-04-12 Thread Saam Barati


> On Apr 12, 2018, at 5:21 AM, Yusuke SUZUKI  wrote:
> 
> Hi, WebKittens, in particular, JSC folks,
> 
> Recently I largely removed duplicate 32bit DFG code by unifying the 
> implementation of 32bit and 64bit. Initially, we have separate code for 32bit 
> and 64bit. But as our DFG infrastructure grows, we have fancy DFG features 
> that enables writing unified implementation for 32bit and 64bit. For example, 
> JSValueRegs families offer the way to handle JSValue registers transparently.
> 
> Unifying the code significantly reduces the size of 
> DFGSpeculativeJIT32_64.cpp. It reduces maintenance burden. In addition, it 
> finds bugs, for example, 32bit code sometimes lacks the changes done in 64bit 
> code (e.g. mutatorFence). This unification finds and fixes such a bug.
mutatorFence isn’t needed on 32-bit since it doesn’t have the concurrent GC.

> 
> So I suggest not adding code to DFGSpeculativeJIT32_64.cpp as much as 
> possible. We can implement large part of DFG code in 32/64 unified code with 
> our infrastructure. If your 32bit code is slightly different from 64bit code, 
> I believe adding ifdef in DFGSpeculativeJIT.cpp is rather fine. It highlights 
> what our infrastructure is missing to implement it in unified code. And 
> later, such ifdefs will be removed.
This seems like a good approach to me.

- Saam
> 
> Best regards,
> Yusuke Suzuki
> ___
> jsc-dev mailing list
> jsc-...@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/jsc-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Growing tired of long build times? Check out this awesome new way to speed up your build... soon (HINT: It's not buying a new computer)

2017-08-29 Thread Saam Barati


> On Aug 29, 2017, at 1:22 PM, Olmstead, Don  wrote:
> 
> Sure but in a corporate environment it can be a really big win. All of our 
> developers have their machines hooked up to the distributed build system and 
> we also have servers hanging out that can be used in the build. This sort of 
> thing doesn't require anyone to change how they write their code which is 
> something a unity build ends up requiring.
> 
> In terms of WebKit development the only places it falls apart are in the 
> source code generation and the link.
> 
> Really I was just curious if anything like that was discussed because all of 
> this requires pretty big changes to source code. Additionally there's other 
> fun things that can happen around includes when doing a unity build where a 
> file will stop being able to be built in isolation. The same thing has 
> happened with WebCorePrefix where a number of files can't be built without it.
Maybe it’d be worth having an EWS bot builds the non-unity build so we don’t 
get into the same situation here.

- Saam
> 
> -Original Message-
> From: Konstantin Tokarev [mailto:annu...@yandex.ru] 
> Sent: Tuesday, August 29, 2017 11:22 AM
> To: Olmstead, Don ; Keith Miller 
> ; WebKit Development Mailing List 
> 
> Subject: Re: [webkit-dev] Growing tired of long build times? Check out this 
> awesome new way to speed up your build... soon (HINT: It's not buying a new 
> computer)
> 
> 
> 
> 29.08.2017, 21:19, "Olmstead, Don" :
>> Did you happen to look at any sort of distributed build system? We use 
>> distributed builds here for WebKit and a full rebuild is pretty reasonable. 
>> That would negate the need to change how everything is done to get a unity 
>> build going.
> 
> This isn't helpful for contributors who have only one machine at their 
> disposal
> 
>> 
>> From: webkit-dev 
>> [mailto:webkit-dev-bounces+don.olmstead=am.sony@lists.webkit.org] On 
>> Behalf Of Keith Miller
>> Sent: Monday, August 28, 2017 5:37 PM
>> To: WebKit Development Mailing List 
>> Subject: [webkit-dev] Growing tired of long build times? Check out this 
>> awesome new way to speed up your build... soon (HINT: It's not buying a new 
>> computer)
>> 
>> Hello fellow Webkittens,
>> 
>> Are you growing tired of long cat naps while waiting 45 minutes for WebKit 
>> to build? (I’ve definitely never done this 🤐!)
>> 
>> Do you want WebKit to build up to 3-4x faster on a clean build?*
>> 
>> Does seeing your incremental build… stay the same sound good?
>> 
>> You’ll be purring with excitement after you switch to unified source 
>> builds!**
>> 
>> * Building JSC with CMake, including CMake configuration time, went from 
>> ~8m15s to ~2m30s on my MBP using unified sources only on cpp files in 
>> JavaScriptCore. Final results on WebKit may vary. Using unified sources may 
>> cause sudden feelings of euphoria and productivity (or grumpiness from lack 
>> of naps…). Not responsible for any bugs in extra patches produced by using 
>> unified source builds (excluding build system related bugs) except where 
>> required by law (pi...@apple.com).
>> 
>> ** Soon. Unified source builds haven’t been landed yet.
>> 
>> How are things going to change? Great question! Let’s find out!
>> 
>> When you fire off the build script it will automagically paw your source 
>> files into bundles. The size of each bundle is speculatively going to be 8 
>> .cpp files but that’s up for experimentation. From my testing so far, the 
>> compile time for a bundle is at most 20% (~6s vs ~7s for the Air directory) 
>> longer than the longest file it includes. Given that most of the build time 
>> in incremental builds is scanning dependencies, this change is probably only 
>> barely noticeable.
>> 
>> Bundling happens as part of the CMake configuration process, in the CMake 
>> build. In the Xcode build it’s more complicated since we cannot dynamically 
>> choose the files we are going to compile. The only solution I know of is to 
>> pre-list a bunch of files for the build to dump files into. Occasionally, 
>> we’ll need to add more files to the build list.
>> 
>> When you add or remove a file in the project, the cost is higher. The 
>> current plan to bundle source files is by directory. This means that when a 
>> .cpp file is added or removed you will rebalance the bundles in its 
>> directory, and you will have to rebuild up to the full directory that file 
>> is in.
>> 
>> My goal is to make all of this invisible to developers as much as possible. 
>> I believe, for the most part, things should operate mostly as they have 
>> before. As the details on unified source builds get finalized, I’m sending 
>> out a separate email documenting any workflow changes. I’ll also do my best 
>> to answer any questions or concerns.
>> 
>> “But Keith, am I going to have to make major changes to the way I develop 
>> WebKit?” Yes and, hopefully, no. There is one major common workflow change 
>> that comes with unified sources, the naming of stati

Re: [webkit-dev] Bring back ARMv6 support to JSC

2017-07-13 Thread Saam barati
And ARES6.

- Saam

> On Jul 13, 2017, at 1:50 PM, Saam barati  wrote:
> 
> Can you please run Octane and Kraken too?
> 
> - Saam
> 
>> On Jul 13, 2017, at 7:47 AM, Caio Lima > <mailto:ticaiol...@gmail.com>> wrote:
>> 
>> Finally I got the results from the last benchmark run. The results
>> shows that the speed-ups are considerable comparing with CLoop
>> version, since we get faster results in a big number of tests and
>> regress in a minor number of scripts. I would like to get feedback
>> from you as well, but IMHO enabling JIT for ARMv6 looks a good
>> improvement step and the amount of code we are touching in current
>> trunk code to make it possible is small.
>> 
>> The results are attached and I also uploaded them in
>> https://bugs.webkit.org/show_bug.cgi?id=172765 
>> <https://bugs.webkit.org/show_bug.cgi?id=172765>.
>> 
>> PS.: Some test cases (bigswitch-indirect-symbol-or-undefined,
>> bigswitch-indirect-symbol, bigswitch, etc) are failing now and I'm
>> already investigating the source of problem to fix them.
>> 
>> Regards,
>> Caio.
>> 
>> 2017-07-05 22:54 GMT-03:00 Filip Pizlo > <mailto:fpi...@apple.com>>:
>>> To be clear, I’m concerned that the 32-bit JIT backends have such bad 
>>> tuning for these embedded platforms that it’s just pure badness. Until you 
>>> can prove that you can change this, I think that porting should focus on 
>>> making the cloop great. Then, we can rip out support for weird CPUs rather 
>>> than bringing it back.
>>> 
>>> -Filip
>>> 
>>>> On Jul 5, 2017, at 6:14 PM, Caio Lima >>> <mailto:ticaiol...@gmail.com>> wrote:
>>>> 
>>>> 2017-07-05 18:25 GMT-03:00 Filip Pizlo >>> <mailto:fpi...@apple.com>>:
>>>>> You need to establish that the JIT is a performance progression over the 
>>>>> LLInt on ARMv6. I am opposed to more ARMv6 patches landing until there is 
>>>>> some evidence provided that you’re actually getting speed-ups.
>>>> 
>>>> It makes sense. I can get these numbers related to JIT.
>>>> 
>>>> BTW, there is a Patch that isn't JIT related
>>>> (https://bugs.webkit.org/show_bug.cgi?id=172766 
>>>> <https://bugs.webkit.org/show_bug.cgi?id=172766>).
>>>> 
>>>> Regards,
>>>> Caio.
>>>> 
>>>>> -Filip
>>>>> 
>>>>>> On Jun 13, 2017, at 6:48 PM, Caio Lima >>>>> <mailto:ticaiol...@gmail.com>> wrote:
>>>>>> 
>>>>>> Hi All.
>>>>>> 
>>>>>> Some of you guys might know me through the work I have been doing in
>>>>>> JSC. The experience working with WebKit has been great so far, thank
>>>>>> you for the reviews!
>>>>>> 
>>>>>> Since 1st May, we at Igalia have been working on bring back the ARMv6
>>>>>> support into JSC. We already have commits into our downstream branch
>>>>>> port[2] that fixes some compile/runtime errors when building JSC to
>>>>>> ARMv6 and also fixes some bugs. However, this branch is not synced
>>>>>> with WebKit upstream tree and I would like to pursue the upstreaming
>>>>>> of this ARMv6/JSC support to WebKit.
>>>>>> 
>>>>>> As a long shot, we are planning to maintain the ARMv6 support and make
>>>>>> tests run as green as possible. Also, it's our goal make ARMv6 support
>>>>>> not interfere with other ARM versions support code negatively and we
>>>>>> will be in charge of implement platform-specific fixes/features for
>>>>>> JSC/ARM6, this way no imposing burden to the rest of the community.
>>>>>> 
>>>>>> To keep track of work to be done, I've create a meta-bug in
>>>>>> bugzilla[3] and it's going to be used firstly to organize the commits
>>>>>> from our downstream branch, but pretty soon I'm going to create issues
>>>>>> related with javascriptcore-test failures and send patches to fix
>>>>>> them. We have already submitted 3 patches (they are marked as
>>>>>> dependence of [3]) that fixes ARMv6 into LLInt and JIT layers and got
>>>>>> a round of review into them.
>>>>>> 
>>>>>> Best Regards,
>>>>>> Caio.
>>>>>> 
>>>>>> [1] - https://www.igalia.com/about-us/coding-experience 
>>>>>> <https://www.igalia.com/about-us/coding-experience>
>>>>>> [2] - https://github.com/WebPlatformForEmbedded/WPEWebKit 
>>>>>> <https://github.com/WebPlatformForEmbedded/WPEWebKit>
>>>>>> [3] - https://bugs.webkit.org/show_bug.cgi?id=172765 
>>>>>> <https://bugs.webkit.org/show_bug.cgi?id=172765>
>>>>>> ___
>>>>>> webkit-dev mailing list
>>>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> <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] Bring back ARMv6 support to JSC

2017-07-13 Thread Saam barati
Can you please run Octane and Kraken too?

- Saam

> On Jul 13, 2017, at 7:47 AM, Caio Lima  wrote:
> 
> Finally I got the results from the last benchmark run. The results
> shows that the speed-ups are considerable comparing with CLoop
> version, since we get faster results in a big number of tests and
> regress in a minor number of scripts. I would like to get feedback
> from you as well, but IMHO enabling JIT for ARMv6 looks a good
> improvement step and the amount of code we are touching in current
> trunk code to make it possible is small.
> 
> The results are attached and I also uploaded them in
> https://bugs.webkit.org/show_bug.cgi?id=172765 
> .
> 
> PS.: Some test cases (bigswitch-indirect-symbol-or-undefined,
> bigswitch-indirect-symbol, bigswitch, etc) are failing now and I'm
> already investigating the source of problem to fix them.
> 
> Regards,
> Caio.
> 
> 2017-07-05 22:54 GMT-03:00 Filip Pizlo  >:
>> To be clear, I’m concerned that the 32-bit JIT backends have such bad tuning 
>> for these embedded platforms that it’s just pure badness. Until you can 
>> prove that you can change this, I think that porting should focus on making 
>> the cloop great. Then, we can rip out support for weird CPUs rather than 
>> bringing it back.
>> 
>> -Filip
>> 
>>> On Jul 5, 2017, at 6:14 PM, Caio Lima  wrote:
>>> 
>>> 2017-07-05 18:25 GMT-03:00 Filip Pizlo :
 You need to establish that the JIT is a performance progression over the 
 LLInt on ARMv6. I am opposed to more ARMv6 patches landing until there is 
 some evidence provided that you’re actually getting speed-ups.
>>> 
>>> It makes sense. I can get these numbers related to JIT.
>>> 
>>> BTW, there is a Patch that isn't JIT related
>>> (https://bugs.webkit.org/show_bug.cgi?id=172766).
>>> 
>>> Regards,
>>> Caio.
>>> 
 -Filip
 
> On Jun 13, 2017, at 6:48 PM, Caio Lima  wrote:
> 
> Hi All.
> 
> Some of you guys might know me through the work I have been doing in
> JSC. The experience working with WebKit has been great so far, thank
> you for the reviews!
> 
> Since 1st May, we at Igalia have been working on bring back the ARMv6
> support into JSC. We already have commits into our downstream branch
> port[2] that fixes some compile/runtime errors when building JSC to
> ARMv6 and also fixes some bugs. However, this branch is not synced
> with WebKit upstream tree and I would like to pursue the upstreaming
> of this ARMv6/JSC support to WebKit.
> 
> As a long shot, we are planning to maintain the ARMv6 support and make
> tests run as green as possible. Also, it's our goal make ARMv6 support
> not interfere with other ARM versions support code negatively and we
> will be in charge of implement platform-specific fixes/features for
> JSC/ARM6, this way no imposing burden to the rest of the community.
> 
> To keep track of work to be done, I've create a meta-bug in
> bugzilla[3] and it's going to be used firstly to organize the commits
> from our downstream branch, but pretty soon I'm going to create issues
> related with javascriptcore-test failures and send patches to fix
> them. We have already submitted 3 patches (they are marked as
> dependence of [3]) that fixes ARMv6 into LLInt and JIT layers and got
> a round of review into them.
> 
> Best Regards,
> Caio.
> 
> [1] - https://www.igalia.com/about-us/coding-experience
> [2] - https://github.com/WebPlatformForEmbedded/WPEWebKit
> [3] - https://bugs.webkit.org/show_bug.cgi?id=172765
> ___
> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Bring back ARMv6 support to JSC

2017-07-05 Thread Saam barati
What’s the testing plan here? Do y'all plan to add a bot that ensures ARMv6 
stays working?

- Saam

> On Jun 13, 2017, at 6:48 PM, Caio Lima  wrote:
> 
> Hi All.
> 
> Some of you guys might know me through the work I have been doing in
> JSC. The experience working with WebKit has been great so far, thank
> you for the reviews!
> 
> Since 1st May, we at Igalia have been working on bring back the ARMv6
> support into JSC. We already have commits into our downstream branch
> port[2] that fixes some compile/runtime errors when building JSC to
> ARMv6 and also fixes some bugs. However, this branch is not synced
> with WebKit upstream tree and I would like to pursue the upstreaming
> of this ARMv6/JSC support to WebKit.
> 
> As a long shot, we are planning to maintain the ARMv6 support and make
> tests run as green as possible. Also, it's our goal make ARMv6 support
> not interfere with other ARM versions support code negatively and we
> will be in charge of implement platform-specific fixes/features for
> JSC/ARM6, this way no imposing burden to the rest of the community.
> 
> To keep track of work to be done, I've create a meta-bug in
> bugzilla[3] and it's going to be used firstly to organize the commits
> from our downstream branch, but pretty soon I'm going to create issues
> related with javascriptcore-test failures and send patches to fix
> them. We have already submitted 3 patches (they are marked as
> dependence of [3]) that fixes ARMv6 into LLInt and JIT layers and got
> a round of review into them.
> 
> Best Regards,
> Caio.
> 
> [1] - https://www.igalia.com/about-us/coding-experience
> [2] - https://github.com/WebPlatformForEmbedded/WPEWebKit
> [3] - https://bugs.webkit.org/show_bug.cgi?id=172765
> ___
> 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] User agent woes

2017-05-07 Thread Saam Barati


> On May 6, 2017, at 5:19 PM, Michael Catanzaro  wrote:
> 
> Hi,
> 
> You're probably aware that WebKitGTK+ has user agent quirks to make various 
> popular websites work, most notably google.com.  For our list of quirks, see:
> 
> https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/UserAgentQuirks.cpp
> 
> Recently we had two major bugs caused by these quirks:
> 
> https://bugs.webkit.org/show_bug.cgi?id=171603 (New YouTube displays only 
> white page)
> https://bugs.webkit.org/show_bug.cgi?id=171770 (Google login fails on white 
> page)
> 
> In the case of the YouTube bug, I decided to just remove the quirk. This case 
> was pretty much a nightmare scenario, because Google decided that a random 
> subset of users would receive the new version of YouTube long in advance of 
> the rest of us. This means that we had a portion of our users complaining 
> that YouTube was "broken" for weeks when we were unable to reproduce the 
> issue (and had no way to do so; how could we have guessed it was some trial 
> program?).
> 
> Anyway, removing the quirk is not a good option for the generic google.com 
> quirk in the second bug. If we use a standard user agent, we receive a crap 
> unusable 1990s version of Google Calendar, a high source of user complaints, 
> and also the awesome 3D earth mode in Google Maps that our users expect is 
> unavailable. This makes users think that WebKit is bad. I found a solution, 
> but I know it's temporary; give it a few more months and Google will break us 
> again, no doubt.
> 
> User agent is an extremely demotivating, never-ending game, and it's by far 
> our biggest web compatibility problem. It almost feels as if Google is 
> deliberately trying to break WebKit, which I know is not true as they don't 
> care either way about us... but they do know full well that basing logic off 
> of user agent checks serves to harm less-popular browsers, so it's hardly 
> unintentional. I cannot think of any aspect of WebKit development less 
> gratifying than maintaining our user agent quirk list, nor any bigger user 
> agent offender than Google.
> 
> For a while I thought there was nothing we could do, but now I have an idea: 
> Safari could adopt the same (or lightly-adapted) user agent quirks that we 
> use for WebKitGTK+. Of course, only the small handful of websites to which we 
> currently need to send user agent quirks would be affected by this change: 
> Google, Yahoo, Slack, Whatsapp, Typekit, and some Chinese site Taobao. Now, 
> this would do no good for Safari, but it would be a huge help to us as it 
> would ensure that if these user agent offenders attempt to degrade 
> functionality for browsers not on their lists, they will have to at least 
> treat all WebKit browsers equally. Presumably they test to make sure their 
> websites work in Safari, so that should make this situation much better for 
> other ports. And if we run into problems, we at least know the change is 
> limited to this small set of websites.
> 
> I think the existing quirks would be fine for Safari with minimal changes. We 
> would definitely need to add some #if OS(UNIX) && !PLATFORM(COCOA) guards 
> inside urlRequiresLinuxDesktopPlatform(), and put if !PLATFORM(IOS) around 
> the quirks that are designed to turn off mobile versions of websites. This 
> would only be a small amount of work to set up, and it would only affect the 
> handful of websites that we have identified as problematic. Would Apple be 
> willing to allow this?
> 
> Alternatively, we could use the nuclear option and add a Chrome version 
> component, similar to how Chrome includes a Safari version in its user agent. 
> (Bonus: that will shut up Google's "switch to Chrome" ads for a couple weeks 
> until they figure it out.) Edge already includes a Chrome version. This would 
> undoubtedly be better for the web as a whole, but it would surely also 
> introduce serious short-term compatibility problems, as an unknown number of 
> websites would likely break horribly in Safari (and some known websites, e.g. 
> YouTube and Google Hangouts), so that's a pretty tough ask. Making it more 
> difficult for websites to send us crap versions of web pages would be good 
> for WebKit in the long term, but it's way safer to start only on quirks for 
> specific sites.
> 
> What do you think?
This is not my area of expertise, but why can't you just adopt Safari's user 
agent instead of the other way around?

- Saam

> 
> Michael
> 
> ___
> 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] !!Tests for equality comparison

2017-04-27 Thread Saam Barati

> On Apr 27, 2017, at 4:30 PM, Geoffrey Garen  wrote:
> 
> I’ve never really liked this style rule, and I’ve always felt like it snuck 
> into the style document without much discussion.
> 
> Even so, I usually tolerate it.

I’m also not a big fan of this rule.

- Saam

> 
> Geoff
> 
>> On Apr 27, 2017, at 4:06 PM, JF Bastien > > wrote:
>> 
>> Hello C++ fans!
>> 
>> The C++ style check currently say:
>> Tests for true/false, null/non-null, and zero/non-zero should all be done 
>> without equality comparisons
>> 
>> I totally agree for booleans and pointers… but not for integers. I know it’s 
>> pretty much the same thing, but I it takes me slightly longer to process 
>> code like this:
>> 
>> int numTestsForEqualityComparison = 0:
>> // Count ‘em!
>> // …
>> if (!numTestsForEqualityComparison)
>>   printf(“Good job!”);
>> 
>> I read it as “if not number of tests for equality comparison”. That's weird. 
>> It takes me every slightly longer to think about, and I’ve gotten it wrong a 
>> bunch of times already. I’m not trying to check for “notness", I’m trying to 
>> say “if there were zero tests for equality comparison”, a.k.a.:
>> 
>> if (numTestsForEqualityComparison == 0)
>>   printf(“Good job!”);
>> 
>> So how about the C++ style let me just say that? I’m not suggesting we 
>> advise using that style for integers everywhere, I’m just saying it should 
>> be acceptable to check zero/non-zero using equality comparison.
>> 
>> 
>> !!Thanks (i.e. many thanks),
>> 
>> JF
>> 
>> p.s.: With you I am, fans of Yoda comparison, but for another day this will 
>> be.
>> ___
>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Why does RELEASE_ASSERT not have an error message?

2017-02-22 Thread Saam barati

> On Feb 22, 2017, at 12:16 PM, Filip Pizlo  wrote:
> 
> 
>> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen  wrote:
>> 
>> I’ve lost countless hours to investigating CrashTracers that would have been 
>> easy to solve if I had access to register state.
> 
> The current RELEASE_ASSERT means that every assertion in what the compiler 
> thinks is a function (i.e. some function and everything inlined into it) is 
> coalesced into a single trap site.  I’d like to understand how you use the 
> register state if you don’t even know which assertion you are at.
When I disassemble JavaScriptCore, I often find a succession of int3s at the 
bottom of a function. Does LLVM sometimes combine them and sometimes not?

For example, this is what the bottom of the 
__ZN3JSC20AbstractModuleRecord18getModuleNamespaceEPNS_9ExecStateE looks like:

5c25popq%r14
5c27popq%r15
5c29popq%rbp
5c2aretq
5c2bint3
5c2cint3
5c2dint3
5c2eint3
5c2fnop

- Saam
> 
> I believe that if you do want to analyze register state, then switching back 
> to calling some function that prints out diagnostic information is strictly 
> better.  Sure, you get less register state, but at least you know where you 
> crashed.  Knowing where you crashed is much more important than knowing the 
> register state, since the register state is not useful if you don’t know 
> where you crashed.
> 
>> 
>> I also want the freedom to add RELEASE_ASSERT without ruining performance 
>> due to bad register allocation or making the code too large to inline. For 
>> example, hot paths in WTF::Vector use RELEASE_ASSERT.
> 
> Do we have data about the performance benefits of the current RELEASE_ASSERT 
> implementation?
> 
>> 
>> Is some compromise solution possible?
>> 
>> Some options:
>> 
>> (1) Add a variant of RELEASE_ASSERT that takes a string and logs.
> 
> The point of C++ assert macros is that I don’t have to add a custom string.  
> I want a RELEASE_ASSERT macro that automatically stringifies the expression 
> and uses that as the string.
> 
> If I had a choice between a RELEASE_ASSERT that can accurate report where it 
> crashed but sometimes trashes the register state, and a RELEASE_ASSERT that 
> always gives me the register state but cannot tell me which assert in the 
> function it’s coming from, then I would always choose the one that can tell 
> me where it crashed.  That’s much more important, and the register state is 
> not useful without that information.
> 
>> 
>> (2) Change RELEASE_ASSERT to do the normal debug ASSERT thing in Debug 
>> builds. (There’s not much need to preserve register state in debug builds.)
> 
> That would be nice, but doesn’t make RELEASE_ASSERT useful for debugging 
> issues where timing is important.  I no longer use RELEASE_ASSERTS for those 
> kinds of assertions, because if I do it then I will never know where I 
> crashed.  So, I use the explicit:
> 
> if (!thing) {
>   dataLog(“…”);
>   RELEASE_ASSERT_NOT_REACHED();
> }
> 
> -Filip
> 
> 
>> 
>> Geoff
>> 
>>> On Feb 22, 2017, at 11:09 AM, Filip Pizlo  wrote:
>>> 
>>> I disagree actually.  I've lost countless hours to converting this:
>>> 
>>> RELEASE_ASSERT(blah)
>>> 
>>> into this:
>>> 
>>> if (!blah) {
>>> dataLog("Reason why I crashed");
>>> RELEASE_ASSERT_NOT_REACHED();
>>> }
>>> 
>>> Look in the code - you'll find lots of stuff like this.
>>> 
>>> I don't think analyzing register state at crashes is more important than 
>>> keeping our code sane.
>>> 
>>> -Filip
>>> 
>>> 
>>>> On Feb 21, 2017, at 5:56 PM, Mark Lam  wrote:
>>>> 
>>>> Oh yeah, I forgot about that.  I think the register state is more 
>>>> important for crash analysis, especially if we can make sure that the 
>>>> compiler does not aggregate the int3s.  I’ll explore alternatives.
>>>> 
>>>>> On Feb 21, 2017, at 5:54 PM, Saam barati  wrote:
>>>>> 
>>>>> I thought the main point of moving to SIGTRAP was to preserve register 
>>>>> state?
>>>>> 
>>>>> That said, there are probably places where we care more about the message 
>>>>> than the registers.
>>>>> 
>>>>> - Saam
>>>>> 
>>>>>> On Feb 21, 2017, at 5:43 PM, Mark Lam  wrot

Re: [webkit-dev] Why does RELEASE_ASSERT not have an error message?

2017-02-22 Thread Saam barati
I like this idea. I think even doing what JF suggested could be really nice for 
suspected crashes. That way the register state would just tell us the reason.
If we do log, I think we should use WTFLogAlways instead of dataLog, because I 
think that does show up in some crash logs (but I could be wrong about this, 
I’m not 100% sure).

- Saam

> On Feb 22, 2017, at 11:58 AM, Geoffrey Garen  wrote:
> 
> I’ve lost countless hours to investigating CrashTracers that would have been 
> easy to solve if I had access to register state.
> 
> I also want the freedom to add RELEASE_ASSERT without ruining performance due 
> to bad register allocation or making the code too large to inline. For 
> example, hot paths in WTF::Vector use RELEASE_ASSERT.
> 
> Is some compromise solution possible?
> 
> Some options:
> 
> (1) Add a variant of RELEASE_ASSERT that takes a string and logs.
> 
> (2) Change RELEASE_ASSERT to do the normal debug ASSERT thing in Debug 
> builds. (There’s not much need to preserve register state in debug builds.)
> 
> Geoff
> 
>> On Feb 22, 2017, at 11:09 AM, Filip Pizlo  wrote:
>> 
>> I disagree actually.  I've lost countless hours to converting this:
>> 
>> RELEASE_ASSERT(blah)
>> 
>> into this:
>> 
>> if (!blah) {
>>  dataLog("Reason why I crashed");
>>  RELEASE_ASSERT_NOT_REACHED();
>> }
>> 
>> Look in the code - you'll find lots of stuff like this.
>> 
>> I don't think analyzing register state at crashes is more important than 
>> keeping our code sane.
>> 
>> -Filip
>> 
>> 
>>> On Feb 21, 2017, at 5:56 PM, Mark Lam  wrote:
>>> 
>>> Oh yeah, I forgot about that.  I think the register state is more important 
>>> for crash analysis, especially if we can make sure that the compiler does 
>>> not aggregate the int3s.  I’ll explore alternatives.
>>> 
>>>> On Feb 21, 2017, at 5:54 PM, Saam barati  wrote:
>>>> 
>>>> I thought the main point of moving to SIGTRAP was to preserve register 
>>>> state?
>>>> 
>>>> That said, there are probably places where we care more about the message 
>>>> than the registers.
>>>> 
>>>> - Saam
>>>> 
>>>>> On Feb 21, 2017, at 5:43 PM, Mark Lam  wrote:
>>>>> 
>>>>> Is there a reason why RELEASE_ASSERT (and friends) does not call 
>>>>> WTFReportAssertionFailure() to report where the assertion occur?  Is this 
>>>>> purely to save memory?  svn blame tells me that it has been this way 
>>>>> since the introduction of RELEASE_ASSERT in r140577 many years ago.
>>>>> 
>>>>> Would anyone object to adding a call to WTFReportAssertionFailure() in 
>>>>> RELEASE_ASSERT() like we do for ASSERT()?  One of the upside 
>>>>> (side-effect) of adding this call is that it appears to stop the compiler 
>>>>> from aggregating all the RELEASE_ASSERTS into a single code location, and 
>>>>> this will help with post-mortem crash debugging.
>>>>> 
>>>>> Any thoughts?
>>>>> 
>>>>> Mark
>>>>> 
>>>>> ___
>>>>> 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 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Why does RELEASE_ASSERT not have an error message?

2017-02-21 Thread Saam barati
I thought the main point of moving to SIGTRAP was to preserve register state?

That said, there are probably places where we care more about the message than 
the registers.

- Saam

> On Feb 21, 2017, at 5:43 PM, Mark Lam  wrote:
> 
> Is there a reason why RELEASE_ASSERT (and friends) does not call 
> WTFReportAssertionFailure() to report where the assertion occur?  Is this 
> purely to save memory?  svn blame tells me that it has been this way since 
> the introduction of RELEASE_ASSERT in r140577 many years ago.
> 
> Would anyone object to adding a call to WTFReportAssertionFailure() in 
> RELEASE_ASSERT() like we do for ASSERT()?  One of the upside (side-effect) of 
> adding this call is that it appears to stop the compiler from aggregating all 
> the RELEASE_ASSERTS into a single code location, and this will help with 
> post-mortem crash debugging.
> 
> Any thoughts?
> 
> Mark
> 
> ___
> 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] [webkit-reviewers] usage of auto

2017-01-17 Thread Saam barati

> On Jan 12, 2017, at 10:37 AM, Geoffrey Garen  wrote:
> 
>>> e.g. I think this is great:
>>> auto ptr = std::make_unique(bar);
>>> Proposed rule: if the type is obvious because it's on the line, then auto 
>>> is good.
>>> Similarly:
>>> auto i = static_cast(j);
>>> auto foo = make_foo();
>>> auto bar = something.get_bar(); // Sometimes, "bar" is obvious.
>> I'm not sure I agree with this style. There are times where the type of an 
>> auto variable is obvious-enough, but it's almost never more obvious than 
>> actually writing out the types.
> 
> In the first case, static_cast, the type is written out on the same 
> line. Same goes for other casts, make_unique, and so on. Would you accept 
> auto in those cases? If not, what benefit do you get from seeing the type 
> twice on one line?

My reason I don't like this style is it slightly changes how I read code. I 
typically look at the lhs of an expression to get its type.
Now, I may have to look at the lhs, and maybe at some compound expression on 
the rhs. I just don’t see the point of this
to save a few characters.

For cases like jsDynamicCast, std::make_unique, etc, I also have to know what 
those functions do. This is obvious to experienced JSC/C++ developers, but 
maybe not to all newcomers to the code base.

All that said, this usage of auto probably bothers me the least out of all the 
usages I don’t like.

- Saam

> 
> I think the second and third cases are somewhat rare in WebKit, and I might 
> agree against using auto. For example:
> 
> RefPtr thing;
> auto* context = something.context();
> context->setThing(thing.get()); // I need to research why context->thing is 
> allowed to be a raw pointer, but I don’t know what context is.
> 
> Geoff
> ___
> 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] [webkit-reviewers] usage of auto

2017-01-11 Thread saam barati


> On Jan 11, 2017, at 11:15 AM, JF Bastien  wrote:
> 
> Would it be helpful to focus on small well-defined cases where auto makes 
> sense, and progressively grow that list as we see fit?
> 
> 
> e.g. I think this is great:
> auto ptr = std::make_unique(bar);
> Proposed rule: if the type is obvious because it's on the line, then auto is 
> good.
> Similarly:
> auto i = static_cast(j);
> auto foo = make_foo();
> auto bar = something.get_bar(); // Sometimes, "bar" is obvious.
I'm not sure I agree with this style. There are times where the type of an auto 
variable is obvious-enough, but it's almost never more obvious than actually 
writing out the types. Writing out types, for my brain at least, almost always 
makes the code easier to understand. The most obvious place where I prefer auto 
over explicit types is when something has a lot of template bloat.

I feel like the places where auto makes the code better are limited, but places 
where auto makes the code more confusing, or requires me to spend more time 
figuring it out, are widespread. (Again, this is how my brain reads code.)

Also, I completely agree with Geoff that I use types to grep around the source 
code and to figure out what data structures are being used. If we used auto 
more inside JSC it would hurt my workflow for reading and understanding new 
code.

- Saam

> 
> 
> Range-based loops are a bit tricky. IMO containers with "simple" types are 
> good candidates for either:
> for (const auto& v : cont) { /* don't change v */ }
> for auto& v : cont) { /* change v */ }
> But what's "simple"? I'd say all numeric, pointer, and string types at least. 
> It gets tricky for more complex types, and I'd often rather have the type in 
> the loop. Here's more discussion on this, including a recommendation to use 
> auto&& on range-based loops! I think this gets confusing, and I'm not a huge 
> fan of r-value references everywhere.
> 
> 
> Here's another I like, which Yusuke pointed out a while ago (in ES6 Module's 
> implementation?):
> struct Foo {
>   typedef Something Bar;
>   // ...
>   Bar doIt();
> };
> auto Foo::doIt() -> Bar
> {
>   // ...
> }
> Why? Because Bar is scoped to Foo! It looks odd the first time, but I think 
> this is idiomatic "modern" C++.
> 
> 
> I also like creating unnamed types, though I know this isn't everyone's 
> liking:
> auto ohMy()
> {
>   struct { int a; float b; } result;
>   // ...
>   return result;
> }
> void yeah()
> {
>   auto myMy = ohMy();
>   dataLogLn(myMy.a, myMy.b);
> }
> I initially had that with consumeLoad, which returns a T as well as a 
> ConsumeDependency. I couldn't care less about the container for T and 
> ConsumeDependency, I just want these two values.
> ___
> 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] Naming preference: SetForScope vs. TemporaryChange

2016-12-26 Thread Saam Barati
Right. I see what you're saying. The name doesn't confuse me with respect to 
these semantics but I see that's it's subtly wrong.

The use case I was thinking of is this:
`
class Foo {
Foo() : m_change(someIntVariable, 20) { }
...
...
ScopedChange m_change;
};
`

Which is admittedly an odd use case that probably doesn't exist in WebKit. 
However, if this use case were common, the name ScopedChange feels wrong for 
it. 

- Saam

> On Dec 25, 2016, at 7:29 PM, Maciej Stachowiak  wrote:
> 
> 
>> On Dec 25, 2016, at 11:35 AM, Saam Barati  wrote:
>> 
>> I like ScopedChange the most out of all the names. It's a bit unfortunate 
>> that such a name doesn't work well in the context of having a ScopedChange 
>> as a member variable. I think TemporaryChange works much better as a name if 
>> the use is as a member variable.
>> 
>> My hunch would be the most frequent use of this class is as a scoped change. 
>> If almost all of the uses are indeed for this, my name preference would be:
>> 1. ScopedChange
>> 2. TemporaryChange
>> 
>> If there are a lot of uses where the class isn't used as a scoped change, my 
>> preference would be to revert to TemporaryChange.
> 
> I'm not sure what distinction you are trying to draw between "scoped change" 
> and "temporary change". It seems pretty clear that this class is meant to be 
> used as a value object on the stack, so anything it does is scoped.
> 
> The distinction I was trying to draw is that, if you make additional changes 
> to the value still within the scope of the object, they will be overwritten. 
> That aspect isn't really captured by either "ScopedChange" or 
> "TemporaryChange". That's because, in some fundamental underlying sense, 
> what's really scoped is the restore of the original value, not the change. 
> 
> Maybe a concrete example would help show what I'm taking about:
> 
> // global
> double tachyonFlux = 3.14;
> 
> void redirectEnginesToShield()
> {
>ScopedChange(tachyonFlux, 2.0);
> 
>doOneThing();
> 
>tachyonFlux = 1.0;
> 
>doAnotherThing();
> 
>// on scope exit, both the scopedChange and the explicit assignment are 
> undone
> }
> 
> Perhaps this doesn't matter in practice because there's never actually 
> additional assignments in the scope of one of these things so it will be 
> clear enough as actually used.
> 
> Regards,
> Maciej
> 
> 
>> 
>> - Saam
>> 
>>> On Dec 23, 2016, at 6:50 AM, Maciej Stachowiak  wrote:
>>> 
>>> 
>>> A few more coats of paint for the bike shed:
>>> 
>>> It's a little unusual to have a class name that's a verb phrase instead of 
>>> a noun phrase. And in this case if you interpret "Set" as a noun you'll get 
>>> entirely the wrong idea. Some alternatives that avoid this, but has the 
>>> better clarity of "Scope" instead of "Temporary" would be "ScopedChange or 
>>> "ScopedAssignment".
>>> 
>>> One additional thing to think about: the class doesn't just have the effect 
>>> of limiting the assignment to a scope. It will also undo any further 
>>> assignments to the reference it holds that happen until it is destroyed. 
>>> Save-restore semantics like this are common but often the names involved 
>>> highlight the restore rather than the setting. I can't think of a great 
>>> name off the top of my head but something like RestoreOnScopeExit seems 
>>> more technically accurate than SetForScope.
>>> 
>>> - Maciej
>>> 
>>>> On Dec 23, 2016, at 6:32 AM, Michael Catanzaro  
>>>> wrote:
>>>> 
>>>> On Fri, 2016-12-23 at 05:42 +, Yusuke SUZUKI wrote:
>>>>> Personally I like the name "SetForScope" since the name "scope"
>>>>> states that this value change is tied to C++ scope.
>>>> 
>>>> Me too. The name is pretty clear. The first time I saw TemporaryChange
>>>> I had to look at the implementation to see what it did.
>>>> 
>>>> Michael
>>>> ___
>>>> 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] Naming preference: SetForScope vs. TemporaryChange

2016-12-25 Thread Saam Barati
I like ScopedChange the most out of all the names. It's a bit unfortunate that 
such a name doesn't work well in the context of having a ScopedChange as a 
member variable. I think TemporaryChange works much better as a name if the use 
is as a member variable.

My hunch would be the most frequent use of this class is as a scoped change. If 
almost all of the uses are indeed for this, my name preference would be:
1. ScopedChange
2. TemporaryChange

If there are a lot of uses where the class isn't used as a scoped change, my 
preference would be to revert to TemporaryChange.

- Saam

> On Dec 23, 2016, at 6:50 AM, Maciej Stachowiak  wrote:
> 
> 
> A few more coats of paint for the bike shed:
> 
> It's a little unusual to have a class name that's a verb phrase instead of a 
> noun phrase. And in this case if you interpret "Set" as a noun you'll get 
> entirely the wrong idea. Some alternatives that avoid this, but has the 
> better clarity of "Scope" instead of "Temporary" would be "ScopedChange or 
> "ScopedAssignment".
> 
> One additional thing to think about: the class doesn't just have the effect 
> of limiting the assignment to a scope. It will also undo any further 
> assignments to the reference it holds that happen until it is destroyed. 
> Save-restore semantics like this are common but often the names involved 
> highlight the restore rather than the setting. I can't think of a great name 
> off the top of my head but something like RestoreOnScopeExit seems more 
> technically accurate than SetForScope.
> 
> - Maciej
> 
>> On Dec 23, 2016, at 6:32 AM, Michael Catanzaro  wrote:
>> 
>> On Fri, 2016-12-23 at 05:42 +, Yusuke SUZUKI wrote:
>>> Personally I like the name "SetForScope" since the name "scope"
>>> states that this value change is tied to C++ scope.
>> 
>> Me too. The name is pretty clear. The first time I saw TemporaryChange
>> I had to look at the implementation to see what it did.
>> 
>> Michael
>> ___
>> 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] Naming preference: SetForScope vs. TemporaryChange

2016-12-22 Thread Saam barati
Hi all,

Recently, Yusuke found that JSC and WTF both had the exact same RAII helper 
data structure. JSC called it SetForScope, and WTF called it TemporaryChange. 
(Details here: https://bugs.webkit.org/show_bug.cgi?id=164761 
). Yusuke went to replace JSC’s 
use of SetForScope with TemporaryChange. When I reviewed his patch, I suggested 
to him to rename the class to SetForScope because it was my personal preference 
of the two names. However, I should have first discussed this change with other 
WebKit developers (so I’m doing that now, retroactively).

If there is a strong preference to use the name TemporaryChange instead of 
SetForScope, I’ll rename the class back to its original name.

My personal preference is still for the name SetForScope, but my feelings are 
not strongly tied to one name or the other.

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


Re: [webkit-dev] SIGSEGV possibly (?) due to libwebkitgtk (backtrace included)

2016-07-04 Thread Saam Barati
Looks like JSValue() (which is the empty JS Value, with bit representation as 
zero) is showing up somewhere it shouldn't be. It's hard to tell what the bug 
is just from your stack trace. It could be helpful to enable/disable various 
JIT tiers to see if it still reproduces and take it from there. It would also 
be helpful to identify the crashing function so we can see its source code and 
its byte code.

Presumably this is an older version of JSC, so whatever bug you're seeing might 
be fixed in ToT. 

- Saam

> On Jul 3, 2016, at 4:11 PM, Scott Kostyshak  wrote:
> 
> Dear all,
> 
> I am getting a SIGSEGV from a package that depends on
> libwebkitgtk-3.0-0, gnome-web-photo (note that I am on Ubuntu 16.04).
> 
> The following command is what gives me the SIGSEGV:
> gnome-web-photo "http://www.nba.com"; "gwp_test.png"
> 
> it only happens with that website.
> 
> Here is the backtrace I have:
> 
> Core was generated by `gnome-web-photo http://www.nba.com gwp_test.png'.
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  JSC::JSCell::getPrimitiveNumber (this=this@entry=0x0, 
> exec=exec@entry=0x7f135c6ccbe0, number=@0x7ffee6aa2060: 0, value=...)
>at ../Source/JavaScriptCore/runtime/JSCell.cpp:134
> 134 ../Source/JavaScriptCore/runtime/JSCell.cpp: No such file or 
> directory.
> [Current thread is 1 (Thread 0x7f13cb26da80 (LWP 2386))]
> (gdb) bt
> #0  JSC::JSCell::getPrimitiveNumber (this=this@entry=0x0, 
> exec=exec@entry=0x7f135c6ccbe0, number=@0x7ffee6aa2060: 0, value=...)
>at ../Source/JavaScriptCore/runtime/JSCell.cpp:134
> #1  0x7f13c6ae4bdc in JSC::JSValue::getPrimitiveNumber (value=..., 
> number=@0x7ffee6aa2060: 0, exec=0x7f135c6ccbe0, this=)
>at ../Source/JavaScriptCore/runtime/JSCJSValueInlines.h:599
> #2  JSC::jsLess (v2=..., v1=..., callFrame=0x7f135c6ccbe0) at 
> ../Source/JavaScriptCore/runtime/Operations.h:136
> #3  JSC::slow_path_less (exec=0x7f135c6ccbe0, pc=0x7f130d4e51e8) at 
> ../Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:295
> #4  0x7f1366ed165b in ?? ()
> #5  0x in ?? ()
> (gdb)
> 
> Best,
> 
> Scott
> ___
> 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] Keith Miller is now a WebKit reviewer

2016-04-05 Thread Saam Barati
Congrats!

Saam

> On Apr 5, 2016, at 12:32 PM, Mark Lam  wrote:
> 
> Hi everyone,
> 
> Just want to announce that Keith Miller is now a reviewer.  Keith primarily 
> works on JavaScriptCore.  Feel free to ask him for reviews.
> 
> Congratulations, Keith.
> 
> Mark
> 
> ___
> 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] Sukolsak Sakshuwong is now a WebKit Reviewer

2016-03-08 Thread Saam barati
Congrats Sukol!

Saam

> On Mar 8, 2016, at 6:33 PM, Yusuke SUZUKI  wrote:
> 
> Congrats!
> 
> ---
> Regards,
> Yusuke Suzuki
> 
> On Wed, Mar 9, 2016 at 6:01 AM, Ryosuke Niwa  > wrote:
> Congratulations, Sukolsak!
> 
> - R. Niwa
> 
> On Tue, Mar 8, 2016 at 11:33 AM, Mark Lam  > wrote:
> Hi everyone,
> 
> Just want to announce that Sukolsak Sakshuwong is now a reviewer.  
> Congratulations, Sukolsak.
> 
> Mark
> 
> ___
> 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 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] Should overridden methods use 'virtual' keyword in addition to 'override'?

2016-03-03 Thread Saam barati
(3) seems like a good idea to me!
I’ll volunteer for (1)

Saam
> On Mar 3, 2016, at 11:35 AM, Darin Adler  wrote:
> 
> 3) physically restrain me from turning do-webcore-rename into a perl script 
> that does this all the code in the entire source tree all at one go, since 
> that would be a bad idea, right?

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


Re: [webkit-dev] Should overridden methods use 'virtual' keyword in addition to 'override'?

2016-03-03 Thread Saam barati
+1.
I like how “override” only reads.

Saam

> On Mar 3, 2016, at 9:54 AM, Ryosuke Niwa  wrote:
> 
> I think "virtual" + "override" is more of a historical artifact than
> the preferred style because we used to have OVERRIDE macro before all
> compilers supported C++11.  I think we should just use only "override"
> going forward.
> - R. Niwa
> 
> 
> On Thu, Mar 3, 2016 at 9:38 AM, Darin Adler  wrote:
>> Antti proposed using only “override” a while back since it’s less verbose 
>> and still unambiguous. I don’t think we reached consensus on which style to 
>> prefer for the project, though.
>> 
>> — Darin
>> ___
>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Which bugzilla component should be used for bmalloc?

2016-02-23 Thread saam barati
I also came across this when doing a bmalloc patch recently. I think it'd be 
useful to have a bmalloc component.  

Saam

> On Feb 22, 2016, at 7:58 AM, Konstantin Tokarev  wrote:
> 
> Hello,
> 
> It seems like there is no appropriate bugzilla component for bmalloc. In my 
> understanding, closest one is JavaScriptCore since it is mostly work on by 
> the same folks who work on JSC, but technically they are very different 
> components.
> 
> I think we need to create new component "Bmalloc" (or "FastMalloc" to 
> abstract away of current implementation).
> 
> -- 
> Regards,
> Konstantin
> ___
> 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] Some text about the B3 compiler

2016-01-29 Thread saam barati
I'm happy to convert the document to markdown. Can you send me your latest 
revision or post it to the website?

I usually look at:
http://daringfireball.net/projects/markdown/syntax
For a refresher on the syntax.

Tim, could you create a page that loads the markdown file?

Thanks,
Saam

> On Jan 29, 2016, at 12:06 AM, Filip Pizło  wrote:
> 
> I'm all for this but I don't know anything about markdown. 
> 
> What's the best way to proceed?
> 
> -Filip
> 
>> On Jan 28, 2016, at 9:24 PM, Timothy Hatcher  wrote:
>> 
>> They should be markdown files like we do for the code style and policy 
>> documents.
>> 
>> https://trac.webkit.org/browser/trunk/Websites/webkit.org/code-style.md
>> 
>> We can then make Wordpress pages on the site that load the markdown.
>> 
>> Maybe put them in a /docs/b3/ directory?
>> 
>> — Timothy Hatcher
>> 
>>> On Jan 28, 2016, at 4:48 PM, Filip Pizlo  wrote:
>>> 
>>> I guess we could put it in Websites/webkit.org/b3.  Then patches could edit 
>>> both B3 and the documentation in one go, and the documentation would go 
>>> live when it’s committed.
>>> 
>>> Does anyone object to this?
>>> 
>>> -Filip
>>> 
>>> 
>>>> On Jan 28, 2016, at 4:39 PM, Saam barati  wrote:
>>>> 
>>>> Yeah. That’d be the easiest way to keep it up IMO.
>>>> 
>>>> Saam
>>>> 
>>>>> On Jan 28, 2016, at 4:37 PM, Filip Pizło  wrote:
>>>>> 
>>>>> +1
>>>>> 
>>>>> Do you think we should move the documentation to a file in svn so that it 
>>>>> can be reviewed as part of patch review?
>>>>> 
>>>>> -Filip
>>>>> 
>>>>>> On Jan 28, 2016, at 4:32 PM, Saam barati  wrote:
>>>>>> 
>>>>>> This is great. Thanks Fil.
>>>>>> 
>>>>>> I propose that we do all that we can to keep this updated.
>>>>>> I suggest that all patches that change to the IR should also include 
>>>>>> with it 
>>>>>> a change to the documentation, and that reviewers should require this.
>>>>>> 
>>>>>> It’d also be great if other significant changes that seem like the 
>>>>>> deserve
>>>>>> a mention in the documentation also get added as part of patches.
>>>>>> 
>>>>>> Saam
>>>>>> 
>>>>>>> On Jan 28, 2016, at 4:23 PM, Filip Pizlo  wrote:
>>>>>>> 
>>>>>>> Hi everyone,
>>>>>>> 
>>>>>>> We’ve been working on a new compiler backend for the FTL JIT, which we 
>>>>>>> call B3.  It stands for “Bare Bones Backend”.  We recently enabled it 
>>>>>>> on X86/Mac, and we’re working hard to enable it on other platforms.
>>>>>>> 
>>>>>>> If you’re interested in how it works, I’ve started writing 
>>>>>>> documentation.  I’ll be adding more to it soon!
>>>>>>> https://trac.webkit.org/wiki/BareBonesBackend
>>>>>>> https://trac.webkit.org/wiki/B3IntermediateRepresentation
>>>>>>> 
>>>>>>> -Filip
>>>>>>> 
>>>>>>> ___
>>>>>>> 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 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] Some text about the B3 compiler

2016-01-28 Thread Saam barati
Yeah. That’d be the easiest way to keep it up IMO.

Saam

> On Jan 28, 2016, at 4:37 PM, Filip Pizło  wrote:
> 
> +1
> 
> Do you think we should move the documentation to a file in svn so that it can 
> be reviewed as part of patch review?
> 
> -Filip
> 
> On Jan 28, 2016, at 4:32 PM, Saam barati  <mailto:sbar...@apple.com>> wrote:
> 
>> This is great. Thanks Fil.
>> 
>> I propose that we do all that we can to keep this updated.
>> I suggest that all patches that change to the IR should also include with it 
>> a change to the documentation, and that reviewers should require this.
>> 
>> It’d also be great if other significant changes that seem like the deserve
>> a mention in the documentation also get added as part of patches.
>> 
>> Saam
>> 
>>> On Jan 28, 2016, at 4:23 PM, Filip Pizlo >> <mailto:fpi...@apple.com>> wrote:
>>> 
>>> Hi everyone,
>>> 
>>> We’ve been working on a new compiler backend for the FTL JIT, which we call 
>>> B3.  It stands for “Bare Bones Backend”.  We recently enabled it on 
>>> X86/Mac, and we’re working hard to enable it on other platforms.
>>> 
>>> If you’re interested in how it works, I’ve started writing documentation.  
>>> I’ll be adding more to it soon!
>>> https://trac.webkit.org/wiki/BareBonesBackend 
>>> <https://trac.webkit.org/wiki/BareBonesBackend>
>>> https://trac.webkit.org/wiki/B3IntermediateRepresentation 
>>> <https://trac.webkit.org/wiki/B3IntermediateRepresentation>
>>> 
>>> -Filip
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>> <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] Some text about the B3 compiler

2016-01-28 Thread Saam barati
This is great. Thanks Fil.

I propose that we do all that we can to keep this updated.
I suggest that all patches that change to the IR should also include with it 
a change to the documentation, and that reviewers should require this.

It’d also be great if other significant changes that seem like the deserve
a mention in the documentation also get added as part of patches.

Saam

> On Jan 28, 2016, at 4:23 PM, Filip Pizlo  wrote:
> 
> Hi everyone,
> 
> We’ve been working on a new compiler backend for the FTL JIT, which we call 
> B3.  It stands for “Bare Bones Backend”.  We recently enabled it on X86/Mac, 
> and we’re working hard to enable it on other platforms.
> 
> If you’re interested in how it works, I’ve started writing documentation.  
> I’ll be adding more to it soon!
> https://trac.webkit.org/wiki/BareBonesBackend 
> 
> https://trac.webkit.org/wiki/B3IntermediateRepresentation 
> 
> 
> -Filip
> 
> ___
> 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] Getting the frame pointer and program counter of a thread on gtk and efl ports

2016-01-06 Thread Saam barati
Hi all,
I’m writing some platform dependent code for a sampling profiler I’m adding to 
JSC.
I’m looking for a way to get a thread’s frame pointer and program counter in 
the gtk and efl ports while that thread is paused.
We already have a way to grab the stack pointer of a thread on all ports. If 
you look inside JavaScriptCore/heap/MachineStackMarker.h/cpp you
will see the code that does this. I’ve also written code that grabs PC and FP 
for other platforms. If you look at my WIP patch
you can see that code: https://bugs.webkit.org/show_bug.cgi?id=151713 
.

I’d like to get this working on all ports before landing. Do people who 
maintain the gtk and efl ports
have an idea on how to do this? Or could you help me figure out a solution?

Thanks,
Saam___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Contributing to webkit Open Source project

2015-08-03 Thread saam barati
Hi Mukesh,

I agree with Geoff that working ES6 features is a great starting point.
Also, going on #webkit IRC is helpful and you can speak with other people
working on JSC/WebKit.
If you have any questions, my IRC handle is saamyjoon.


On Mon, Aug 3, 2015 at 11:14 AM, Geoffrey Garen  wrote:

> Hi Mukesh.
>
> Welcome to the project.
>
> How did your project turn out? If it was successful, the best place to
> start is to post patches to bugs.webkit.org.
>
> Another important project we’re working on right now is finishing our
> implementation of ES6. A quick way to get a list of missing features is to
> navigate to  using a WebKit
> nightly build.
>
> Regards,
> Geoff
>
> On Aug 2, 2015, at 3:12 PM, Mukesh Kumar Tiwari  wrote:
>
> Hi,
>
> I am graduate from Cornell willing to contribute to WebKit Open Source
> project. During the college, I have worked on a project to optimize LLVM
> backend of the Webkit and have a decent knowledge of the FTL Layer. I would
> like to gain more knowledge by contributing to WebKit Project. Please guide
> me on where to begin with.
>
> Thanks and Regards,
> *Mukesh Kumar Tiwari*
> *Software Development Engineer*
> *Amazon.com *
> ___
> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] WebCore/platform standalone library

2015-03-19 Thread saam barati
I think JavaScriptCore will need access to 'platform', too, to implement some 
ES6 features. 




Saam

On Thu, Mar 19, 2015 at 2:50 PM, Maciej Stachowiak  wrote:

>> On Mar 19, 2015, at 1:47 PM, Benjamin Poulain  wrote:
>> 
>> On 3/18/15 9:43 PM, Myles C. Maxfield wrote:
>>> Hello, all,
>>> 
>>> I’d like to announce that I intend to create a standalone static library 
>>> from the current contents of WebCore/platform over the coming months. This 
>>> will involve creating a “Platform" top-level directory and moving source 
>>> files into it, one by one.
>>> 
>>> There are a few reasons for this:
>>> 
>>> 1. Enforcing the layering between Platform and WebCore. Moving Platform 
>>> into its own target/directory can guarantee that nothing inside it knows 
>>> about anything in WebCore.
>>> 2. Being able to test code in the Platform directory with TestWebKitAPI 
>>> (without exporting Platform symbols from the WebCore library)
>>> 3. Managing conceptual complexity.
>>> 
>>> Does anyone have any thoughts or feedback?
>> 
>> That's an awesome project. That's gonna be a lot of work.
>> 
>> How do you plan to do the interface between WebCore and Platform?
>> 
>> Between WebCore and WebKit, we use interfaces with pure virtual functions 
>> that are implemented by the clients.
>> Between WebCore and the platform, we have headers and each port has its own 
>> implementation of that interface.
>> 
>> Do you plan to move Platform behind a public interface or keep the current 
>> model?
> I don’t think we need a model like the WebCore/WebKit interface. WTF is 
> essentially like the proposed Platform library already, and it just exposes 
> normal C++ headers and implementation files. I think the main benefit here is 
> cleaning up the layering, as opposed to adding more abstraction. In fact, you 
> could sort of think of WTF and Platform as logically the same library, with 
> WTF being only the parts needed by JavaScriptCore, plus things that are 
> logically at the same level (so basically non-GUI and no networking code).
> This almost makes me want to suggest a jokey name for Platform. I can’t off 
> the top of my head think of a good expansion of OMG, though. Or BBQ.
> Regards,
> Maciej
> ___
> 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