Skip to content

Commit e244b8b

Browse files
authored
Memoised the container probe and de-duplicated the contextualize dispatch. (#104)
1 parent 6f6bd80 commit e244b8b

8 files changed

Lines changed: 95 additions & 55 deletions

File tree

benchmarks/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ Reference points from the CI runner (Linux, PHP 8.3) - the same environment the
2525

2626
Peak memory is ~2 MB across all subjects.
2727

28-
## Optimization opportunities
28+
## Optimization notes
2929

30-
Candidates for a dedicated performance pass, with the evidence the suite surfaces:
30+
What the suite surfaced, and where it landed:
3131

32-
1. **Native-host detection cost.** `benchDetectDrupalNative` (~41 μs) is dramatically slower than `benchDetectDrupalContainer` (~12 μs) despite the container doing more contextualization work. The native path pays for `Container::isContainer()`'s filesystem probes (`file_exists('/.dockerenv')`, `file_exists('/.dockerinit')`, `is_readable('/proc/1/cgroup')`) and, during contextualization, the reflection fallback in `AbstractStack::contextualize()` / `AbstractPlatform::contextualize()` (`Native` is not a `DrupalContextualizerInterface`, so a method name is built via `ReflectionClass::getShortName()`). The container path short-circuits `isContainer()` on an env var and dispatches through the typed fast path. This gap is the suite's clearest optimization target.
33-
2. **`is()` re-reads the env var.** `Environment::is()` calls `getenv('ENVIRONMENT_TYPE')` on every invocation even though the type is already resolved. Caching it in a static property realizes the documented "statically cached" design and speeds the repeated-check path.
34-
3. **Duplicated dispatch.** `AbstractPlatform::contextualize()` and `AbstractStack::contextualize()` are identical; the shared dispatch can move to a trait.
32+
1. **Container probing ran per stack.** Every container-family stack (Ddev, Lando, Container) inherits `Container::isContainer()`, so a single native-host detection re-ran the same env/filesystem probe up to three times. `Container::isContainer()` now memoises its result for the run (cleared on `Environment::reset()`), so the probe runs once while a subclass overriding `isContainer()` still opts out. This cut `benchDetectDrupalNative` by ~19% locally (more on Linux, where the probe actually reads `/proc/1/cgroup`). The remaining single probe is inherent to detecting containerisation.
33+
2. **Duplicated dispatch.** `AbstractPlatform::contextualize()` and `AbstractStack::contextualize()` were byte-identical; they now share the `DispatchesContextualization` trait, which dispatches the built-in Drupal context through its typed interface and falls back to reflection only for custom contexts.
34+
3. **`is()` reads the env var, by design.** `Environment::is()` calls `getenv('ENVIRONMENT_TYPE')` each time. This is deliberate: the env var is the single source of truth for the type - both the override input and the published output. Caching it in a static would create a second store that can silently diverge, so the per-call `getenv()` stays.
3535

3636
## Comparison and baseline
3737

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DrevOps\EnvironmentDetector;
6+
7+
use DrevOps\EnvironmentDetector\Contexts\ContextInterface;
8+
use DrevOps\EnvironmentDetector\Contexts\Drupal;
9+
use DrevOps\EnvironmentDetector\Contexts\DrupalContextualizerInterface;
10+
11+
/**
12+
* Dispatches a context to the matching per-context method on a ring.
13+
*
14+
* Shared by platforms and stacks so both apply context-specific settings the
15+
* same way.
16+
*
17+
* @package DrevOps\EnvironmentDetector
18+
*/
19+
trait DispatchesContextualization {
20+
21+
/**
22+
* {@inheritdoc}
23+
*/
24+
public function contextualize(ContextInterface $context): void {
25+
// The built-in Drupal context is dispatched through the typed interface so
26+
// the common path never pays for reflection; a ring that does not
27+
// contextualize Drupal is simply a no-op here.
28+
if ($context instanceof Drupal) {
29+
if ($this instanceof DrupalContextualizerInterface) {
30+
$this->contextualizeDrupal($context);
31+
}
32+
33+
return;
34+
}
35+
36+
// A custom context falls back to a contextualize<ContextName>() method
37+
// resolved from its short name, when the ring defines one.
38+
$method = 'contextualize' . (new \ReflectionClass($context))->getShortName();
39+
$callable = [$this, $method];
40+
41+
if (is_callable($callable)) {
42+
$callable($context);
43+
}
44+
}
45+
46+
}

src/Environment.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,11 @@ public static function reset(bool $all = FALSE): void {
367367
static::$contexts = NULL;
368368
static::$isInitialized = FALSE;
369369

370+
// The container probe caches its result for the run; clearing it lets the
371+
// next detection re-probe, which a test simulating a different host relies
372+
// on.
373+
Container::resetCache();
374+
370375
if ($all) {
371376
static::$fallback = self::DEVELOPMENT;
372377
}

src/Platforms/AbstractPlatform.php

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
namespace DrevOps\EnvironmentDetector\Platforms;
66

7-
use DrevOps\EnvironmentDetector\Contexts\ContextInterface;
8-
use DrevOps\EnvironmentDetector\Contexts\Drupal;
9-
use DrevOps\EnvironmentDetector\Contexts\DrupalContextualizerInterface;
7+
use DrevOps\EnvironmentDetector\DispatchesContextualization;
108

119
/**
1210
* Abstract platform.
@@ -17,6 +15,8 @@
1715
*/
1816
abstract class AbstractPlatform implements PlatformInterface {
1917

18+
use DispatchesContextualization;
19+
2020
/**
2121
* Platform ID. Platforms should override this constant.
2222
*/
@@ -34,26 +34,4 @@ public function id(): string {
3434
return static::ID;
3535
}
3636

37-
/**
38-
* {@inheritdoc}
39-
*/
40-
public function contextualize(ContextInterface $context): void {
41-
// Known interfaces - optimised for speed.
42-
if ($this instanceof DrupalContextualizerInterface && $context instanceof Drupal) {
43-
// Only the most-derived contextualizeDrupal() runs, so a subclass that
44-
// overrides it must call parent::contextualizeDrupal() to keep the
45-
// parent's settings.
46-
$this->contextualizeDrupal($context);
47-
return;
48-
}
49-
50-
// Runtime.
51-
$method = 'contextualize' . (new \ReflectionClass($context))->getShortName();
52-
$callable = [$this, $method];
53-
54-
if (is_callable($callable)) {
55-
$callable($context);
56-
}
57-
}
58-
5937
}

src/Stacks/AbstractStack.php

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
namespace DrevOps\EnvironmentDetector\Stacks;
66

7-
use DrevOps\EnvironmentDetector\Contexts\ContextInterface;
8-
use DrevOps\EnvironmentDetector\Contexts\Drupal;
9-
use DrevOps\EnvironmentDetector\Contexts\DrupalContextualizerInterface;
7+
use DrevOps\EnvironmentDetector\DispatchesContextualization;
108

119
/**
1210
* Abstract stack.
@@ -17,6 +15,8 @@
1715
*/
1816
abstract class AbstractStack implements StackInterface {
1917

18+
use DispatchesContextualization;
19+
2020
/**
2121
* Stack ID. Stacks should override this constant.
2222
*/
@@ -29,26 +29,4 @@ public function id(): string {
2929
return static::ID;
3030
}
3131

32-
/**
33-
* {@inheritdoc}
34-
*/
35-
public function contextualize(ContextInterface $context): void {
36-
// Known interfaces - optimised for speed.
37-
if ($this instanceof DrupalContextualizerInterface && $context instanceof Drupal) {
38-
// Only the most-derived contextualizeDrupal() runs, so a subclass that
39-
// overrides it must call parent::contextualizeDrupal() to keep the
40-
// parent's settings.
41-
$this->contextualizeDrupal($context);
42-
return;
43-
}
44-
45-
// Runtime.
46-
$method = 'contextualize' . (new \ReflectionClass($context))->getShortName();
47-
$callable = [$this, $method];
48-
49-
if (is_callable($callable)) {
50-
$callable($context);
51-
}
52-
}
53-
5432
}

src/Stacks/Container.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ class Container extends AbstractStack implements DrupalContextualizerInterface {
2626
*/
2727
public const SERVICE_HOSTS = ['web', 'app', 'webserver', 'nginx', 'apache', 'apache2'];
2828

29+
/**
30+
* Cached result of the container probe, shared across the run.
31+
*/
32+
protected static ?bool $cachedIsContainer = NULL;
33+
2934
/**
3035
* {@inheritdoc}
3136
*/
@@ -40,6 +45,28 @@ public function active(): bool {
4045
* TRUE if running inside a container, FALSE otherwise.
4146
*/
4247
public function isContainer(): bool {
48+
// Containerisation is fixed for the lifetime of the process, so the probe
49+
// runs once and the result is shared across every container-family stack
50+
// that inherits this method (Ddev, Lando and the generic container would
51+
// otherwise each re-run it). A subclass overriding isContainer() opts out
52+
// of the cache and is probed on its own terms.
53+
return self::$cachedIsContainer ??= $this->detectContainer();
54+
}
55+
56+
/**
57+
* Reset the cached container probe so the next detection re-runs it.
58+
*/
59+
public static function resetCache(): void {
60+
self::$cachedIsContainer = NULL;
61+
}
62+
63+
/**
64+
* Probe the host for the signals that indicate a container.
65+
*
66+
* @return bool
67+
* TRUE if running inside a container, FALSE otherwise.
68+
*/
69+
protected function detectContainer(): bool {
4370
// No single marker reliably proves containerisation across runtimes, so
4471
// probe several independent signals in turn: env vars set by tooling, the
4572
// engine-created marker files, then the control group of PID 1.

tests/Platforms/AbstractPlatformTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@
66

77
use DrevOps\EnvironmentDetector\Contexts\Drupal;
88
use DrevOps\EnvironmentDetector\Contexts\DrupalContextualizerInterface;
9+
use DrevOps\EnvironmentDetector\DispatchesContextualization;
910
use DrevOps\EnvironmentDetector\Platforms\AbstractPlatform;
1011
use DrevOps\EnvironmentDetector\Tests\EnvironmentDetectorTestCase;
1112
use DrevOps\EnvironmentDetector\Tests\Fixtures\NotDrupalContext;
1213
use PHPUnit\Framework\Attributes\CoversClass;
14+
use PHPUnit\Framework\Attributes\CoversTrait;
1315

1416
#[CoversClass(AbstractPlatform::class)]
17+
#[CoversTrait(DispatchesContextualization::class)]
1518
final class AbstractPlatformTest extends EnvironmentDetectorTestCase {
1619

1720
public function testIdDefaultsToConst(): void {

tests/Stacks/AbstractStackTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@
66

77
use DrevOps\EnvironmentDetector\Contexts\Drupal;
88
use DrevOps\EnvironmentDetector\Contexts\DrupalContextualizerInterface;
9+
use DrevOps\EnvironmentDetector\DispatchesContextualization;
910
use DrevOps\EnvironmentDetector\Stacks\AbstractStack;
1011
use DrevOps\EnvironmentDetector\Tests\EnvironmentDetectorTestCase;
1112
use DrevOps\EnvironmentDetector\Tests\Fixtures\NotDrupalContext;
1213
use PHPUnit\Framework\Attributes\CoversClass;
14+
use PHPUnit\Framework\Attributes\CoversTrait;
1315

1416
#[CoversClass(AbstractStack::class)]
17+
#[CoversTrait(DispatchesContextualization::class)]
1518
final class AbstractStackTest extends EnvironmentDetectorTestCase {
1619

1720
public function testIdDefaultsToConst(): void {

0 commit comments

Comments
 (0)