Skip to content

Commit

Permalink
allow to inhibit pinba usage to same cpu and memory as much as possible
Browse files Browse the repository at this point in the history
  • Loading branch information
gggeek committed Dec 12, 2022
1 parent 54bb9b5 commit 91e4c27
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 1 deletion.
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
v1.1 - 12/12/2022

* improved: added new ini setting `pinba.inhibited` which drastically reduces time and memory usage when you want
to really disable all measurement overhead while leaving code instrumented
* improved: fixed memory consumption measurement in `benchmark.php`

v1.0 - 10/12/2022

* improved: replicate extension behaviour: convert tag values to string upon setting them
Expand Down
30 changes: 30 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,25 @@ A trivial usage example can be found in [doc/sample.php](doc/sample.php).
For viewing the gathered metrics, check out https://github.com/intaro/pinboard, https://github.com/pinba-server/pinba-server
or https://github.com/ClickHouse-Ninja/Proton

### Extensions to the original API

#### Pinba::ini_set

If the pinba php extension is not enabled in your setup (which is most likely the case, as otherwise you would not
be using this package), it is not possible from php code to modify the values for ini options `pinba.enabled` and
`pinba.server`. While it is possible to set their value in `php.ini`, if you want to modify their value at runtime you
will have instead to use methods `\PinbaPhp\Polyfill\pinba::ini_set($option, $value)`. You should also use corresponding
method `\PinbaPhp\Polyfill\pinba::ini_get($option)` to check it.

### ini option `pinba.inhibited`

In case you want to keep your code instrumented with `pinba_timer_add`, `pinba_timer_stop` and similar calls but are
not collecting the pinba data anymore, and you want to reduce as much as possible the overhead imposed by this package,
please set in `pinba.inhibited=1` `php.ini`.

Using `pinba.enabled=0` or `pinba.auto_flush=0` is not recommended in that scenario, as, while they both disable the sending
of data, they do not prevent timers to be actually created.

## Compatibility

We strive to implement the same API as Pinba extension ver. 1.1.2.
Expand Down Expand Up @@ -79,9 +98,20 @@ No timing: 0.00001 secs, 0 bytes used
Pinba-extension: 0.00072 secs, 280.640 bytes used
PHP-Pinba: 0.00062 secs, 412.920 bytes used
```

NB: weirdly enough, the php extension seems to be slightly slower on average than the pure-php implementations. Having
taken a cursory look at the C code of the extension, I suspect this is because it executes too many `gettimeofday` calls...

In case you want to keep your code instrumented with lots of `pinba_timer_start` calls, and reduce the overhead of using
the extension as much as possible (while of course not measuring anything anymore), you can set `pinba.inhibited=1` in php.ini.

With that set, this is the overhead you can expect for "timing" 1000 executions of a function call:

```
No timing: 0.00001 secs, 0 bytes used
PHPPinba timed: 0.00009 secs, 0 bytes used
```

(tests executed with php 7.4 in an Ubuntu Focal container, running within an Ubuntu Jammy VM with 4 vCPU allocated)

## Notes
Expand Down
10 changes: 10 additions & 0 deletions src/Pinba.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class Pinba
protected $tags = array();

protected static $options = array();
protected static $inhibited = null;

/// Make this class "abstract", in a way that subclasses can still instantiate it, but no one else can
protected function __construct() {
Expand Down Expand Up @@ -390,6 +391,15 @@ protected static function _send($server, $message)
return false;
}

protected static function isInhibited()
{
if (self::$inhibited === null) {
self::$inhibited = (bool)self::ini_get('pinba.inhibited');
}

return self::$inhibited;
}

/**
* Sadly it is not possible to set in php code values for 'pinba.enabled', at least when the pinba extension is
* not on board. When using `php -d pinba.enabled=1` or values in php.ini, `ini_get` will also not work, whereas
Expand Down
15 changes: 15 additions & 0 deletions src/PinbaClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,12 @@ public function setRusage($rusage)

public function setTag($name, $value)
{
if (self::$inhibited === true || self::isInhibited()) {
return false;
}

$this->tags[$name] = (string)$value;
return true;
}

public function setTimer($tags, $value, $rusage = array(), $hit_count = 1)
Expand All @@ -98,6 +103,10 @@ public function addTimer($tags, $value, $rusage = array(), $hit_count = 1)

protected function upsertTimer($add, $tags, $value, $rusage = array(), $hit_count = 1)
{
if (self::$inhibited === true || self::isInhibited()) {
return '';
}

if (!is_array($tags)) {
trigger_error("setTimer() expects parameter 1 to be array, " . gettype($tags) . " given", E_USER_WARNING);
return false;
Expand Down Expand Up @@ -134,6 +143,8 @@ protected function upsertTimer($add, $tags, $value, $rusage = array(), $hit_coun
"deleted" => false
);
}

return $tagsHash;
}

/**
Expand All @@ -142,6 +153,10 @@ protected function upsertTimer($add, $tags, $value, $rusage = array(), $hit_coun
*/
public function send($flags = null)
{
if (self::$inhibited === true || self::isInhibited()) {
return '';
}

if (!count($this->servers)) {
return false;
}
Expand Down
23 changes: 22 additions & 1 deletion src/PinbaFunctions.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class PinbaFunctions extends Pinba
*/
public static function timer_start($tags, $data = null, $hit_count = 1)
{
// we return 0 to make it possible to later call other `timer_` functions on this without warnings
if (self::$inhibited === true || self::isInhibited()) {
return 0;
}

if (!is_array($tags)) {
trigger_error("pinba_timer_start() expects parameter 1 to be array, " . gettype($tags) . " given", E_USER_WARNING);
return false;
Expand Down Expand Up @@ -64,6 +69,10 @@ public static function timer_start($tags, $data = null, $hit_count = 1)
*/
public static function timer_stop($timer)
{
if (self::$inhibited === true || self::isInhibited()) {
return false;
}

if (!is_int($timer)) {
trigger_error("pinba_timer_stop() expects parameter 1 to be int, " . gettype($timer) . " given", E_USER_WARNING);
return false;
Expand Down Expand Up @@ -93,6 +102,11 @@ public static function timer_stop($timer)
*/
public static function timer_add($tags, $value, $data = null, $hit_count = 1)
{
// we return 0 to make it possible to later call other `timer_` functions on this without warnings
if (self::$inhibited === true || self::isInhibited()) {
return 0;
}

if (!is_array($tags)) {
trigger_error("pinba_timer_add() expects parameter 1 to be array, " . gettype($tags) . " given", E_USER_WARNING);
return false;
Expand Down Expand Up @@ -265,7 +279,6 @@ public static function timers_stop()
*/
public static function timers_get($flag = 0)
{
$time = microtime(true);
$out = array();
$i = self::instance();
foreach($i->timers as $id => $t) {
Expand Down Expand Up @@ -365,6 +378,10 @@ public static function request_time_set($request_time)
*/
public static function tag_set($tag, $value)
{
if (self::$inhibited === true || self::isInhibited()) {
return false;
}

if ($value === "") {
trigger_error("tag name cannot be empty", E_USER_WARNING);
return false;
Expand Down Expand Up @@ -427,6 +444,10 @@ public static function hostname_set($hostname)
*/
public static function flush($script_name = null, $flags = 0)
{
if (self::$inhibited === true || self::isInhibited()) {
return false;
}

$i = self::instance();

// replicate behaviour of php ext: stop running timers even if pinba.enabled is set to false
Expand Down

0 comments on commit 91e4c27

Please sign in to comment.