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

Adding command line parameter to specify the number of HHVM Server Th… #80

Merged
merged 9 commits into from
Mar 22, 2017
15 changes: 14 additions & 1 deletion base/PerfOptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ final class PerfOptions {
public ?string $scriptBeforeWarmup;
public ?string $scriptAfterWarmup;
public ?string $scriptAfterBenchmark;
public ?string $serverThreads;
public ?string $clientThreads;

public bool $notBenchmarking = false;

Expand Down Expand Up @@ -166,6 +168,8 @@ public function __construct(Vector<string> $argv) {
'daemon-files', // daemon output goes to files in the temp directory
'temp-dir:', // temp directory to use; if absent one in /tmp is made
'src-dir:', // location for source to copy into tmp dir instead of ZIP
'server-threads:',
'client-threads:',
};
$targets = $this->getTargetDefinitions()->keys();
$def->addAll($targets);
Expand Down Expand Up @@ -291,7 +295,17 @@ public function __construct(Vector<string> $argv) {
$this->daemonOutputToFile = $this->getBool('daemon-files');

$argTempDir = $this->getNullableString('temp-dir');

if(array_key_exists('server-threads', $o)){
$this->serverThreads = $this->args['server-threads'];
$this->hhvmExtraArguments[count($this->hhvmExtraArguments)] = '-dhhvm.server.thread_count='.$this->serverThreads;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serverThreads should be set by default to 100, and it should be removed from the config files https://github.com/hhvm/oss-performance/blob/master/conf/php.ini#L20. Probably don't use the hhvm extra args for this, and just add another argument in https://github.com/hhvm/oss-performance/blob/master/base/HHVMDaemon.php#L70

}

if(array_key_exists('client-threads', $o)){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs a default.

$this->clientThreads = $this->args['client-threads'];
PerfSettings::setBenchmarkConcurrency((int)$this->clientThreads);
}

if ($argTempDir === null) {
$this->tempDir = tempnam('/tmp', 'hhvm-nginx');
// Currently a file - change to a dir
Expand All @@ -302,7 +316,6 @@ public function __construct(Vector<string> $argv) {
}

$this->srcDir = $this->getNullableString('src-dir');

}

public function validate() {
Expand Down
7 changes: 6 additions & 1 deletion base/PerfSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
final class PerfSettings {

///// Benchmark Settings /////
private static int $benchmarkConcurrency = 200;

// Per concurrent thread - so, total number of requests made during warmup
// is WarmupRequests * WarmupConcurrency
Expand All @@ -28,8 +29,12 @@ public static function BenchmarkTime(): string {
return '1M'; // 1 minute
}

public static function setBenchmarkConcurrency(int $toSet): void {
PerfSettings::$benchmarkConcurrency = $toSet;
}

public static function BenchmarkConcurrency(): int {
return 200;
return PerfSettings::$benchmarkConcurrency;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we actually remove benchmark concurrency settings here, and use the options instead in base/Siege.php? (So basically just make it similar to the way the serverThreads option works)
I think it will end up cleaner.

}

///// Server Settings /////
Expand Down