[webkit-reviews] review granted: [Bug 205279] Changed jsc shell timeout mechanism to leverage the VMTraps and use CPUTime. : [Attachment 385775] proposed patch.

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 205279: Changed jsc shell timeout mechanism to leverage the VMTraps and use
CPUTime.
https://bugs.webkit.org/show_bug.cgi?id=205279

Attachment 385775: proposed patch.

https://bugs.webkit.org/attachment.cgi?id=385775&action=review




--- Comment #8 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 385775
  --> https://bugs.webkit.org/attachment.cgi?id=385775
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=385775&action=review

r=me

Let's please also file a bug for changing our timeouts.

> Source/JavaScriptCore/jsc.cpp:2453
> +    Thread::create("jsc Timeout Thread", [=] () {
> +	   sleep(remainingTime);
> +	   VMInspector::forEachVM([&] (VM& vm) -> VMInspector::FunctorStatus {
> +	       if (&vm != s_vm)
> +		   return VMInspector::FunctorStatus::Continue;
> +	       vm.notifyNeedShellTimeoutCheck();
> +	       return VMInspector::FunctorStatus::Done;
> +	   });
> +    });

This can be a helper shared here and in startTimeoutThreadIfNeeded

> Source/JavaScriptCore/jsc.cpp:2477
> +	   sleep(timeoutDuration / 5.0);

As we talked about in person, let's remove the / 5.0 that you used for testing