phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine
http://pecl.php.net/package/v8js
MIT License
1.84k stars 200 forks source link

Memory / time limit #7

Closed andrewtch closed 11 years ago

andrewtch commented 11 years ago

Hi,

I'm planning to use your library for triggers / data processing, and it would be nice to have time limiting (and probably memory limiting, which is not that important).

This can be passed to executeString as an array of options after $flags parameter. Also, I can try to pullrequest if you can point where to change, but my C knowledge is not that good )

satoshi75nakamoto commented 11 years ago

@andrewtch — I'll take a stab at it later this week. But, feel free to try as well.

andrewtch commented 11 years ago

Digging in, I could not find any time limits for v8 engine itself, so maybe it's not possible at all.

beest commented 11 years ago

@andrewtch you can use PHP's set_time_limit to achieve the same result. It's not the most elegant solution but it does work.

If you need to do any post-timeout clean-up then take a look at register_shutdown_function, which does get triggered after a timeout.

Here's an example:

function shutdown()
{
    print('Shutting down' . PHP_EOL);
}

register_shutdown_function('shutdown');
set_time_limit(1);

$code = <<<EOT

var text = "testing";
for (var i = 0; i < 10000000; ++i) {
    var encoded = encodeURI(text);
}

EOT;

$v8Js = new V8Js();
$v8Js->executeString($code);
andrewtch commented 11 years ago

I'm aware of that. There's no goal to end PHP script after some timeout. The goal is to terminate v8 script and continue PHP script.

beest commented 11 years ago

That's exactly what it does. Give it a try.

andrewtch commented 11 years ago

@beest , no, it isn't.

Assume the following:

foreach ($objects as $object) {
    foreach ($objects->getCallbacks() as $callback) {
        try {
            $v8->executeString($callback->getCode());
        } catch (V8TimeoutException $e) {
              $this->log('callback timeout, callback id: '.$callback->getId());
        }
    }
}

and this is what I actually want.

I do not want to stop the script. I must have all objects and callbacks processed. With your approach I would need to process rest of the objects and callbacks in shutdown function, and what if another script terminates there, do some nesting woodoo with shutdown function?

beest commented 11 years ago

There's nothing available natively in V8 or PHP. You'll need to look at thread synchronization to achieve this.

I'm sure I could put something together to do this, and sounds like @preillyme is going to take a look too. It would indeed be a useful feature.

andrewtch commented 11 years ago

Yep, as I stated above there are no limits in v8 itself. This week I'll try to play with gearman and see how it works with v8, and I'll be much appreciated if you can look on this issue from PHP / V8 side.

MarkOfSine commented 11 years ago

Half-a-tangent... We need similar functionality (i.e Executing a JS script on a backend server) and have looked at various options including spidermonkey and v8js. The challenge we always run into is the timeout issue. Our scripts will be written by external clients and as such we will not be able to vet them to ensure they are well written and do not consume excessive resources or take long to execute.

With this in mind, we are thinking of the following alternative solution: Implement a NodeJs server using DNode RPC (https://github.com/substack/dnode). The JS code is then called via a socket and timeouts can be implemented on both the Php & JS server side.

Js trigger execute

It is obviously more complex to implement but based on the timeout issue inherent with the built-in Php js engines I am not sure how else this could be effectively done.

So, question. Is this overkill, does anyone have an opinion on whether it would work and finally is there perhaps something we have missed with the built-in option that would enable to not go this alternate route?

andrewtch commented 11 years ago

Please take a look on this implementation: http://www.mail-archive.com/v8-users@googlegroups.com/msg04249.html

beest commented 11 years ago

I've got an implementation of timeouts working in my fork: https://github.com/beest/v8js/tree/timeout

It throws a V8JsTimeoutException when a script exceeds a timeout passed in via executeString.

andrewtch commented 11 years ago

maybe it would be better instead of using pthread, which is a big bottleneck in PHP, to use v8::internal::Thread as proposed here: http://pastebin.com/M801zbDB . This would go to native v8 threads instead of messing with phtread library.

beest commented 11 years ago

The v8::internal::Thread class is not in the standard version of the V8 library. The only place I can see it is here: http://blog.peschla.net/doxygen/v8_chromium_r157275/classv8_1_1internal_1_1_thread.html which seems to be a specific implementation of V8 embedded in the Chromium browser. If you have any more information then I'd happily take a look.

The use of pthreads shouldn't inherently be a problem. Note how the sample code you provided links to the pthread library anyway "-lpthread".

However, support for Windows is only provided via a pthreads port. When this version is completed I will look at how to achieve Windows support.

beest commented 11 years ago

I take that back - I can see the Thread class defined in the V8 code base. I'll dig a little deeper and see what I can do with it.

Note that this code also uses pthreads throughout for Linux and OSX. What are the PHP bottlenecks that you're aware of?

andrewtch commented 11 years ago

At old days when I tried to hack PHP code (~5.1) the resources were the problem. For example, if you pass mysql_connect or fopen result to thread (assuming this is a shared connection / file handle), concurrent usage of this resource can/will segfault.

I suppose that the best practice would be to keep the code as is if there's no timeout, and do some woodoo if timeout is requested.

andrewtch commented 11 years ago

Btw, the example above http://pastebin.com/M801zbDB also protects from infinite loops.

beest commented 11 years ago

FYI I've implemented both time and memory limiting in my fork at https://github.com/beest/v8js. It protects from any long-running or memory-intensive code, whether infinite loop or otherwise.

@andrewtch the example you provided was useful and this version works in a very similar way.

@preillyme take a look at the API additions and see if you'd be interested to include this functionality in the official extension.

@MarkOfSine this will give you exactly the functionality that you need.

andrewtch commented 11 years ago

@beest, which branch?

beest commented 11 years ago

@andrewtch master

andrewtch commented 11 years ago

Looks good :)

andrewtch commented 11 years ago

@preillyme , ping!

satoshi75nakamoto commented 11 years ago

@beest — Can you send a PR.

satoshi75nakamoto commented 11 years ago

@beest — ping, can you submit a pull request with your changes.

beest commented 11 years ago

I'll get this done this week. Couple of tweaks required. Sorry for the delay!

satoshi75nakamoto commented 11 years ago

@beest — any luck?

beest commented 11 years ago

Finally got this done. Hope it works out for you, please do give it a try and provide some feedback.

satoshi75nakamoto commented 11 years ago

@beest — Okay I'll take a look. Thanks for all of your hard work.