Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

archive.php compatibility with hhvm #5263

Closed
anonymous-matomo-user opened this issue May 28, 2014 · 7 comments
Closed

archive.php compatibility with hhvm #5263

anonymous-matomo-user opened this issue May 28, 2014 · 7 comments
Assignees
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@anonymous-matomo-user
Copy link

We are tracking 24 sites with piwik, with about 1 million page views per day. Once a day we run archive.php, which runs significant faster with hhvm (Currently about 30 min).

However, there is one compatibility change in piwik/core/CliMulti.php we need to do every time an update to piwik is deployed.

piwik/core/CliMulti.php uses the commandline switch '-q' for building the command to execute (Line 108 in piwik/core/CliMulti.php - Piwik 2.2.2), which is not supported by hhvm.

For our purpose, it's ok to remove '-q' from the command.

@robocoder
Copy link
Contributor

The reason for the -q is because some systems incorrectly use php-cgi (either directly or indirectly, e.g., by symlink).

@robocoder
Copy link
Contributor

In zend emulation mode, hhvm will ignore unknown options. There are two ways to get into zend emulation mode, either:

  • rename/symlink hhvm to php, or
  • invoke hhvm with the --php option

Perhaps change CliMulti.php's findBinary method to use something like:

    private function findPhpBinary()
    {
        if (defined('PHP_BINARY')) {
            if (false === strpos(PHP_BINARY, 'fpm')) {
                if (false === strpos(PHP_BINARY, 'hhvm')) {
                    return PHP_BINARY;
                } else {
                    return PHP_BINARY . ' --php';
                }
            }
        }
...

@mattab
Copy link
Member

mattab commented May 30, 2014

@voidswitch Thanks for the report!

could you please try the change suggested by Anthon, and check it works for you on HHVM without your other patch?

@anonymous-matomo-user
Copy link
Author

hi,

after vacation I changed

private function findPhpBinary()
{
    if (defined('PHP_BINARY') && false === strpos(PHP_BINARY, 'fpm')) {
        return PHP_BINARY;
    }
...

to the implementation suggested by Anthon, and undid the '-q' change. So far it works. Processlist now shows

/usr/bin/hhvm --php -q /srv/nginx/piwik/console climulti:request ....

Edit:
Thank you. More Reports will come, as we plan to use hhvm for the UI. Currently it works fast, but not flawless.

@mattab
Copy link
Member

mattab commented Jun 11, 2014

WE will look forward to your feedback, and hopefully make Piwik work well with HHVM for all users :-)

This ticket is a "sub-task" of #4415 Test if Piwik runs with HipHop Virtual Machine

@mattab
Copy link
Member

mattab commented Jun 13, 2014

In 662c8c4: Fixes #5263 use HHVM binary with --php. Archive may now be compatible with HHVM.

If you still have any issue please comment in the ticket!

@anonymous-matomo-user anonymous-matomo-user added this to the 2.4.0 - Piwik 2.4.0 milestone Jul 8, 2014
@Diftraku
Copy link

Diftraku commented Jul 9, 2014

I am still getting issues with running the archive using the console, removing the -q from the sprintf in CliMulti.php fixes this. It might be useful to have the same checks for HHVM as in CliPhp.php in CliMulti.php for sanity.

# hhvm --version
HipHop VM 3.1.0 (rel)
Compiler: tags/HHVM-3.1.0-0-g71ecbd8fb5e94b2a008387a2b5e9a8df5c6f5c7b
Repo schema: 88ae0db264d72ec2e2eb22ab25d717214aee568b

Piwik 2.4.0

I made small edits to core/CliMulti.php and core/CliMulti/CliPhp.php:

  • removed -q from line 109 in core/CliMulti.php

return sprintf('%s %s/console climulti:request --piwik-domain=%s %s > %s 2>&1 &',

  • added . '-q' to line 22 in core/CliMulti/CliPhp.php

return PHP_BINARY . '-q';

This edit retains backwards compatibility previously used with PHP while allowing HHVM compatibility when -q is not supported.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

4 participants