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

hhvm-repo-mode does not filter files by type #5833

Closed
alex-mashin opened this issue Aug 1, 2015 · 9 comments
Closed

hhvm-repo-mode does not filter files by type #5833

alex-mashin opened this issue Aug 1, 2015 · 9 comments
Assignees
Labels

Comments

@alex-mashin
Copy link

The hhvm-repo-mode script provided by HHVM 3.8.1 is of limited practical use because it does not filter files by type. The script passes all files found in the source directory to hhvm --hphp --target hhbc, even pictures, even files from .git and .svn folders.

As a result, when I ran the script on folder containing three MediaWiki 1.25.1 installations (including their images directories), it took about 80 minutes before I aborted it and consumed 20 GB of RAM. It looks like all the compiled PHP stays in RAM until hhvm --hphp --target hhbc is finished and only then is saved in the repository.

It is also unclear from the documentation (https://github.com/facebook/hhvm/wiki/Performance-Tuning#use-repoauthoritative---20) whether running hhvm-repo-mode script or hhvm --hphp --target hhbc itself on some limited subset of files in project (i.e. the changed ones) will preserve other compiled files in the repository or wipe out the whole of it.

@jwatzman
Copy link
Contributor

jwatzman commented Aug 3, 2015

It was done this way since I didn't want to try to guess what extensions people would be using and so just included everything, but I think .php, .hh, and .inc are far and away the most common (right?) and it would avoid this problem. I'll look into changing it.

will preserve other compiled files in the repository or wipe out the whole of it.

Pretty sure it will always blow the repo away, but should be easy enough to test.

@jwatzman jwatzman self-assigned this Aug 3, 2015
@paulbiss
Copy link
Contributor

paulbiss commented Aug 3, 2015

on some limited subset of files in project

Since repo-authoritative mode heavily relies on whole-program analysis, doing a partial compilation is unlikely to leave the things in a happy state, there is no incremental mode for this type of compilation unfortunately. IIRC we blow the existing repo away anyway.

guess what extensions people would be using

Why not just check if the file has any of the opening tags that hhvm accepts. Realistically you probably only need to search the beginning of the file.

@jwatzman
Copy link
Contributor

jwatzman commented Aug 3, 2015

Why not just check if the file has any of the opening tags that hhvm accepts. Realistically you probably only need to search the beginning of the file.

Is there a nice way to do that in a bash pipeline? :) Maybe with just grep '<?' or something.

@paulbiss
Copy link
Contributor

paulbiss commented Aug 3, 2015

Yeah, that's probably your best bet.

@simonwelsh
Copy link
Contributor

grep -rlE '<?(php|hh|=|[\s])' $PATH instead of find will probably do it. May want to check the short open tag though.

@alex-mashin
Copy link
Author

Since repo-authoritative mode heavily relies on whole-program analysis, doing a partial compilation is unlikely to leave the things in a happy state, there is no incremental mode for this type of compilation unfortunately.
Could an incremental mode, as an option to be used at webmaster's risk, be added?

Because there are frameworks with modular structure, MediaWiki being one of them. It is sad to rebuild the whole repository for one line in LocalSettings.php.

By incremental mode I mean adding one of the two options:

  • one telling the script to recompile the file(s) as instructed without deleting the repository first,
  • one telling the script to recompile only those of the files given that have changed since the last compilation.

@paulbiss
Copy link
Contributor

paulbiss commented Aug 4, 2015

I can understand the utility of such a feature, but adding it would be a major refactor of the way we do static analysis. Right now we inject all manner of assumptions into the bytecode stream based on whole-program analysis. If new modules were introduced that violated these assumptions the runtime would likely crash.

reedy added a commit to reedy/oss-performance that referenced this issue Nov 29, 2015
* Use pristine MediaWiki tarball, copy in LocalSettings.php.
* Remove $wgDisableCounters = true;, code was removed from MW core.
* Remove patch to include LCStoreStaticArray, in MW Core as of 1.26.
* Don't live hack LocalisationCache.php to use $storeClass.
  = 'LCStoreStaticArray';, just set it in LocalSettings.php.
* Update MySQL patch file.
* Add workaround for facebook/hhvm#5833 .
reedy added a commit to wikimedia/mediawiki that referenced this issue Nov 30, 2015
facebook/hhvm#5834
facebook/hhvm#5833

Change-Id: I138ffa5df874c5660897dc7feab36adef9f32aea
reedy added a commit to wikimedia/mediawiki that referenced this issue Dec 2, 2015
facebook/hhvm#5834
facebook/hhvm#5833

Change-Id: I138ffa5df874c5660897dc7feab36adef9f32aea
reedy added a commit to wikimedia/mediawiki that referenced this issue Dec 2, 2015
facebook/hhvm#5834
facebook/hhvm#5833

Change-Id: I138ffa5df874c5660897dc7feab36adef9f32aea
@alex-mashin
Copy link
Author

grep -rlE '<?(php|hh|=|[\s])' $PATH instead of find will probably do it. May want to check the short open tag though.

No, it will not. You need to escape ?.

@Orvid
Copy link
Contributor

Orvid commented Sep 30, 2016

We're a small team, and it doesn't look as though anyone in the community is actively working on this issue, so we've decided to close it. If you plan to send us a pull request resolving the issue let us know in the comments and we will re-open it and assign it to you. If you feel this issue deserves more attention, please comment here with your use-case and we will try to prioritize as appropriate.

@Orvid Orvid closed this as completed Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants