Quantcast

VM::setExclusiveThread()

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

VM::setExclusiveThread()

Mark Lam
The JSC VM has this method setExclusiveThread().  Some details:
1. setExclusiveThread() is only used to forego actually locking/unlocking the underlying lock inside JSLock.
2. setExclusiveThread() is only used by WebCore where we can guarantee that the VM will only ever be used exclusively on one thread.
3. the underlying lock inside JSLock used to be a slow system lock.

Now that we have fast locking, I propose that we simplify the JSLock code by removing the concept of the exclusiveThread and always lock/unlock the underlying lock.  This also give us the ability to tryLock the JSLock (something I would like to be able to do for something new I’m working on).

Does anyone see a reason why we can’t remove the concept of the exclusiveThread?

Thanks.

Mark

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: VM::setExclusiveThread()

Filip Pizlo
Seems like if the relevant benchmarks (speedometer) are ok with it then we should just do this.

-Filip

> On Feb 24, 2017, at 20:50, Mark Lam <[hidden email]> wrote:
>
> The JSC VM has this method setExclusiveThread().  Some details:
> 1. setExclusiveThread() is only used to forego actually locking/unlocking the underlying lock inside JSLock.
> 2. setExclusiveThread() is only used by WebCore where we can guarantee that the VM will only ever be used exclusively on one thread.
> 3. the underlying lock inside JSLock used to be a slow system lock.
>
> Now that we have fast locking, I propose that we simplify the JSLock code by removing the concept of the exclusiveThread and always lock/unlock the underlying lock.  This also give us the ability to tryLock the JSLock (something I would like to be able to do for something new I’m working on).
>
> Does anyone see a reason why we can’t remove the concept of the exclusiveThread?
>
> Thanks.
>
> Mark
>
> _______________________________________________
> webkit-dev mailing list
> [hidden email]
> https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: VM::setExclusiveThread()

Mark Lam
I’ve run Speedometer many times on a quiet 13” MacBookPro: removing the use of exclusive thread status does not appear to impact performance in any measurable way.  I also did some measurements on a microbenchmark locking and unlocking the JSLock using a JSLockHolder in a loop.  The microbenchmark shows that removing exclusive thread status results in the locking and unlocking operation increasing by up to 20%.

Given that locking and unlocking the JSLock is a very small fraction of the work done in a webpage, it’s not surprising that the 20% increase in time for the lock and unlock operation is not measurable in Speedometer.  Note also that the 20% only impacts WebCore which uses the exclusive thread status.  For all other clients of JSC (which never uses exclusive thread status), it may actually be faster to have exclusive thread checks removed (simply due to that code doing less work).

I’ll put up a patch to remove the use of exclusive thread status.  This will simplify the code and make it easier to move forward with new features.

Mark


> On Feb 24, 2017, at 9:01 PM, Filip Pizlo <[hidden email]> wrote:
>
> Seems like if the relevant benchmarks (speedometer) are ok with it then we should just do this.
>
> -Filip
>
>> On Feb 24, 2017, at 20:50, Mark Lam <[hidden email]> wrote:
>>
>> The JSC VM has this method setExclusiveThread().  Some details:
>> 1. setExclusiveThread() is only used to forego actually locking/unlocking the underlying lock inside JSLock.
>> 2. setExclusiveThread() is only used by WebCore where we can guarantee that the VM will only ever be used exclusively on one thread.
>> 3. the underlying lock inside JSLock used to be a slow system lock.
>>
>> Now that we have fast locking, I propose that we simplify the JSLock code by removing the concept of the exclusiveThread and always lock/unlock the underlying lock.  This also give us the ability to tryLock the JSLock (something I would like to be able to do for something new I’m working on).
>>
>> Does anyone see a reason why we can’t remove the concept of the exclusiveThread?
>>
>> Thanks.
>>
>> Mark
>>
>> _______________________________________________
>> webkit-dev mailing list
>> [hidden email]
>> https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: VM::setExclusiveThread()

Filip Pizlo
Sounds good!

I agree that a 20% regression on a microbenchmark of the exclusive JSLock is not a problem, since that's not how WebCore usually behaves and Speedometer doesn't seem to care.

-Filip


> On Feb 28, 2017, at 10:38 AM, Mark Lam <[hidden email]> wrote:
>
> I’ve run Speedometer many times on a quiet 13” MacBookPro: removing the use of exclusive thread status does not appear to impact performance in any measurable way.  I also did some measurements on a microbenchmark locking and unlocking the JSLock using a JSLockHolder in a loop.  The microbenchmark shows that removing exclusive thread status results in the locking and unlocking operation increasing by up to 20%.
>
> Given that locking and unlocking the JSLock is a very small fraction of the work done in a webpage, it’s not surprising that the 20% increase in time for the lock and unlock operation is not measurable in Speedometer.  Note also that the 20% only impacts WebCore which uses the exclusive thread status.  For all other clients of JSC (which never uses exclusive thread status), it may actually be faster to have exclusive thread checks removed (simply due to that code doing less work).
>
> I’ll put up a patch to remove the use of exclusive thread status.  This will simplify the code and make it easier to move forward with new features.
>
> Mark
>
>
>> On Feb 24, 2017, at 9:01 PM, Filip Pizlo <[hidden email]> wrote:
>>
>> Seems like if the relevant benchmarks (speedometer) are ok with it then we should just do this.
>>
>> -Filip
>>
>>> On Feb 24, 2017, at 20:50, Mark Lam <[hidden email]> wrote:
>>>
>>> The JSC VM has this method setExclusiveThread().  Some details:
>>> 1. setExclusiveThread() is only used to forego actually locking/unlocking the underlying lock inside JSLock.
>>> 2. setExclusiveThread() is only used by WebCore where we can guarantee that the VM will only ever be used exclusively on one thread.
>>> 3. the underlying lock inside JSLock used to be a slow system lock.
>>>
>>> Now that we have fast locking, I propose that we simplify the JSLock code by removing the concept of the exclusiveThread and always lock/unlock the underlying lock.  This also give us the ability to tryLock the JSLock (something I would like to be able to do for something new I’m working on).
>>>
>>> Does anyone see a reason why we can’t remove the concept of the exclusiveThread?
>>>
>>> Thanks.
>>>
>>> Mark
>>>
>>> _______________________________________________
>>> webkit-dev mailing list
>>> [hidden email]
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: VM::setExclusiveThread()

Maciej Stachowiak

Good news that it doesn't affect Speedometer. Does this have any effect on pure JS benchmarks running in the browser (e.g. JetStream)?

 - Maciej

> On Feb 28, 2017, at 10:48 AM, Filip Pizlo <[hidden email]> wrote:
>
> Sounds good!
>
> I agree that a 20% regression on a microbenchmark of the exclusive JSLock is not a problem, since that's not how WebCore usually behaves and Speedometer doesn't seem to care.
>
> -Filip
>
>
>> On Feb 28, 2017, at 10:38 AM, Mark Lam <[hidden email]> wrote:
>>
>> I’ve run Speedometer many times on a quiet 13” MacBookPro: removing the use of exclusive thread status does not appear to impact performance in any measurable way.  I also did some measurements on a microbenchmark locking and unlocking the JSLock using a JSLockHolder in a loop.  The microbenchmark shows that removing exclusive thread status results in the locking and unlocking operation increasing by up to 20%.
>>
>> Given that locking and unlocking the JSLock is a very small fraction of the work done in a webpage, it’s not surprising that the 20% increase in time for the lock and unlock operation is not measurable in Speedometer.  Note also that the 20% only impacts WebCore which uses the exclusive thread status.  For all other clients of JSC (which never uses exclusive thread status), it may actually be faster to have exclusive thread checks removed (simply due to that code doing less work).
>>
>> I’ll put up a patch to remove the use of exclusive thread status.  This will simplify the code and make it easier to move forward with new features.
>>
>> Mark
>>
>>
>>> On Feb 24, 2017, at 9:01 PM, Filip Pizlo <[hidden email]> wrote:
>>>
>>> Seems like if the relevant benchmarks (speedometer) are ok with it then we should just do this.
>>>
>>> -Filip
>>>
>>>> On Feb 24, 2017, at 20:50, Mark Lam <[hidden email]> wrote:
>>>>
>>>> The JSC VM has this method setExclusiveThread().  Some details:
>>>> 1. setExclusiveThread() is only used to forego actually locking/unlocking the underlying lock inside JSLock.
>>>> 2. setExclusiveThread() is only used by WebCore where we can guarantee that the VM will only ever be used exclusively on one thread.
>>>> 3. the underlying lock inside JSLock used to be a slow system lock.
>>>>
>>>> Now that we have fast locking, I propose that we simplify the JSLock code by removing the concept of the exclusiveThread and always lock/unlock the underlying lock.  This also give us the ability to tryLock the JSLock (something I would like to be able to do for something new I’m working on).
>>>>
>>>> Does anyone see a reason why we can’t remove the concept of the exclusiveThread?
>>>>
>>>> Thanks.
>>>>
>>>> Mark
>>>>
>>>> _______________________________________________
>>>> webkit-dev mailing list
>>>> [hidden email]
>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
> _______________________________________________
> webkit-dev mailing list
> [hidden email]
> https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: VM::setExclusiveThread()

Filip Pizlo
It would be surprising if it did, since JS benchmarks usually acquire the JS lock around the whole benchmark.

-Filip


> On Feb 28, 2017, at 2:50 PM, Maciej Stachowiak <[hidden email]> wrote:
>
>
> Good news that it doesn't affect Speedometer. Does this have any effect on pure JS benchmarks running in the browser (e.g. JetStream)?
>
> - Maciej
>
>> On Feb 28, 2017, at 10:48 AM, Filip Pizlo <[hidden email]> wrote:
>>
>> Sounds good!
>>
>> I agree that a 20% regression on a microbenchmark of the exclusive JSLock is not a problem, since that's not how WebCore usually behaves and Speedometer doesn't seem to care.
>>
>> -Filip
>>
>>
>>> On Feb 28, 2017, at 10:38 AM, Mark Lam <[hidden email]> wrote:
>>>
>>> I’ve run Speedometer many times on a quiet 13” MacBookPro: removing the use of exclusive thread status does not appear to impact performance in any measurable way.  I also did some measurements on a microbenchmark locking and unlocking the JSLock using a JSLockHolder in a loop.  The microbenchmark shows that removing exclusive thread status results in the locking and unlocking operation increasing by up to 20%.
>>>
>>> Given that locking and unlocking the JSLock is a very small fraction of the work done in a webpage, it’s not surprising that the 20% increase in time for the lock and unlock operation is not measurable in Speedometer.  Note also that the 20% only impacts WebCore which uses the exclusive thread status.  For all other clients of JSC (which never uses exclusive thread status), it may actually be faster to have exclusive thread checks removed (simply due to that code doing less work).
>>>
>>> I’ll put up a patch to remove the use of exclusive thread status.  This will simplify the code and make it easier to move forward with new features.
>>>
>>> Mark
>>>
>>>
>>>> On Feb 24, 2017, at 9:01 PM, Filip Pizlo <[hidden email]> wrote:
>>>>
>>>> Seems like if the relevant benchmarks (speedometer) are ok with it then we should just do this.
>>>>
>>>> -Filip
>>>>
>>>>> On Feb 24, 2017, at 20:50, Mark Lam <[hidden email]> wrote:
>>>>>
>>>>> The JSC VM has this method setExclusiveThread().  Some details:
>>>>> 1. setExclusiveThread() is only used to forego actually locking/unlocking the underlying lock inside JSLock.
>>>>> 2. setExclusiveThread() is only used by WebCore where we can guarantee that the VM will only ever be used exclusively on one thread.
>>>>> 3. the underlying lock inside JSLock used to be a slow system lock.
>>>>>
>>>>> Now that we have fast locking, I propose that we simplify the JSLock code by removing the concept of the exclusiveThread and always lock/unlock the underlying lock.  This also give us the ability to tryLock the JSLock (something I would like to be able to do for something new I’m working on).
>>>>>
>>>>> Does anyone see a reason why we can’t remove the concept of the exclusiveThread?
>>>>>
>>>>> Thanks.
>>>>>
>>>>> Mark
>>>>>
>>>>> _______________________________________________
>>>>> webkit-dev mailing list
>>>>> [hidden email]
>>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>>>
>>
>> _______________________________________________
>> webkit-dev mailing list
>> [hidden email]
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Loading...