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

Program executor #948

Draft
wants to merge 30 commits into
base: master
Choose a base branch
from
Draft

Program executor #948

wants to merge 30 commits into from

Conversation

dantleech
Copy link
Member

@dantleech dantleech commented Nov 21, 2021

This PR experiments with adding a new executor which can execute benchmark variants according to "programs".

A program is a nestable sequence of named "code snippets"/units which are used to compose the benchmarking script.

Benefits:

  • Full flexibility with how the script is executed.
  • Potential to use different strategies (e.g. instead of doing a for loop over the revs, print out the call N times)
  • Potential to add more samplers (not sure what those would be outside of memory/time though)

The default PHPBench executor can be reproduced with:

[
            'before_methods',
            'warmup',
            'hrtime_sampler',
            [
                    'hrtime_sampler',
                    'call_subject',
            ],
            'after_methods',
            'memory_sampler"
]

POC TODO:

  • Create a POC lexer/parser for programs
  • Generate scripts/programs from units
  • Create new PHP process factory / script executor
  • Map script results to classes automatically (support collecting as many different metrics with the same executor)
  • Create the necessary units (hrtime, memory sampler, etc) .
  • Support HRTime

TODO:

  • Is there a viable alternative to the DSL? YES: Replaced with a simple list/object representation
  • Test everything, ensure programs can be constructed safely in any order, enforce cardinality of units (i.e. max 1 of each sampler type)
  • ...

IDEAS:

  • Perhaps opcache can be supported by keeping the benchmarking script(s) for the duration of the run?
  • Support ephermal (file-less) benchmarks, pass the code directly to the PHP process?

/**
* @return array<string,mixed>
*/
public function execute(/** include the run ID here **/string $script): array
Copy link
Member Author

@dantleech dantleech Nov 28, 2021

Choose a reason for hiding this comment

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

the idea was to include the run ID so that it can be used to namespace the file rather than use a uniqid. Doesn't help if we support parallel benchmarking in the future (not sure if that's a valid use case though)

$value = $evaluator->evaluateType($node->node(), NumberNode::class, $params);
$value = $evaluator->evaluate($node->node(), $params);

if ($value instanceof NullNode) {
Copy link
Member Author

Choose a reason for hiding this comment

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

revert

{
public function create(array $data): ResultInterface
{
return new TimeResult(($data['net']), $data['revs'], 'nanoseconds');
Copy link
Member Author

Choose a reason for hiding this comment

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

use TimeUnit constant

Copy link
Member Author

Choose a reason for hiding this comment

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

validate for array key existence

* Return the net-time for this iteration.
*/
public function getNet(): int
{
return $this->netTime;
return (int)$this->netTime;
Copy link
Member Author

@dantleech dantleech Nov 28, 2021

Choose a reason for hiding this comment

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

should be fine to change this to a float? in the medium term we need to change the base from microseconds to nanoseconds. not actually sure what uses this method.


namespace PhpBench\Executor;

class PhpProcessOptions
Copy link
Member Author

Choose a reason for hiding this comment

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

final

$stage = $this->scriptStages[$node->name];
$lines = $this->indent(array_merge([
sprintf('// >>> %s', $node->name),
], $stage->start($context)), $indentation);
Copy link
Member Author

Choose a reason for hiding this comment

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

s/node/unit

@@ -41,6 +41,10 @@ final class Token
public const T_STRING = 'string';
public const T_PARAMETER = 'parameter';
public const T_QUESTION = 'question';
public const T_SEMICOLON = 'semicolon';
public const T_OPEN_BRACE = 'open_brace';
public const T_CLOSE_BRACE = 'close_brace';
Copy link
Member Author

Choose a reason for hiding this comment

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

can remove these now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant