Re: [webkit-dev] VM::setExclusiveThread()
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 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 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 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 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 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 > 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] VM::setExclusiveThread()
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 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 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 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 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 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] VM::setExclusiveThread()
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 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 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 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 >>> 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] VM::setExclusiveThread()
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 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 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 >> 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