Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Commit

Permalink
Add pause to wait for end of warmup. Useful as retranslateAll can run…
Browse files Browse the repository at this point in the history
… long. Also switched options passed to HHVM to be compatible with HHVM 3.0
  • Loading branch information
mofarrell committed Apr 3, 2017
1 parent d4ac30f commit fa84247
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
14 changes: 10 additions & 4 deletions base/HHVMDaemon.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ protected function getTarget(): PerfTarget {
return $this->target;
}

<<__Override>>
public function needsRetranslatePause(): bool {
$status = $this->adminRequest('/warmup-status');
return $status !== '' && $status !== 'failure';
}

<<__Override>>
protected function getArguments(): Vector<string> {
if ($this->options->cpuBind) {
Expand All @@ -82,10 +88,10 @@ protected function getArguments(): Vector<string> {
'Server.ErrorDocument404=index.php',
'-v',
'Server.SourceRoot='.$this->target->getSourceRoot(),
'-d',
'hhvm.log.file='.$this->options->tempDir.'/hhvm_error.log',
'-d',
'pid='.escapeshellarg($this->getPidFilePath()),
'-v',
'Log.File='.$this->options->tempDir.'/hhvm_error.log',
'-v',
'PidFile='.escapeshellarg($this->getPidFilePath()),
'-c',
OSS_PERFORMANCE_ROOT.'/conf/php.ini',
};
Expand Down
2 changes: 2 additions & 0 deletions base/PHPEngine.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@
abstract class PHPEngine extends Process {
public abstract function __toString(): string;
public function writeStats(): void {}

public function needsRetranslatePause(): bool { return false; }
}
5 changes: 5 additions & 0 deletions base/PerfRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ private static function RunWithOptionsAndEngine(
exec($options->scriptAfterWarmup);
}

while ($php_engine->needsRetranslatePause()) {
self::PrintProgress('Pausing 5 seconds to allow retranslation threads to catch up.');
sleep(5);
}

This comment has been minimized.

Copy link
@octmoraru

octmoraru Apr 4, 2017

Contributor

@mofarrell Wouldn't we want this check before the waitAfterWarmup and scriptAfterWarmup checks? Otherwise the scripts hooked through --exec-after-warmup may start before the warmup is actually complete.

This comment has been minimized.

Copy link
@mofarrell

mofarrell Apr 5, 2017

Author Contributor

I have a change for this, it also fires more requests in case we haven't hit the retranslateAll threshold. Ill push it.

self::PrintProgress('Starting Siege for benchmark');
$siege = new Siege($options, $target, RequestModes::BENCHMARK);
$siege->start();
Expand Down

0 comments on commit fa84247

Please sign in to comment.