Page MenuHomePhabricator

API inconsistently uses int casting methods
Closed, ResolvedPublic

Description

Some places we use (int), others we use intval()

We should standardise on using one, and probably just use (int) as it's considered "faster" generally

Event Timeline

Reedy created this task.Feb 24 2019, 10:23 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 24 2019, 10:23 PM

Change 492579 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Use (int) rather than intval()

https://gerrit.wikimedia.org/r/492579

Reedy added a comment.EditedFeb 25 2019, 1:00 AM

What do we do about the array_map( 'intval' calls? Something? Nothing?

According to 3v4l.org

$value = [ "1", "2" ];

$value = array_map( 'intval', $value );

var_dump( $value );

^ takes 58ms

$value = [ "1", "2" ];

$value =  array_map( function( $v ) { return (int)$v; }, $value );

var_dump( $value );

^ takes 59ms

<?php

$value = [ "1", "2" ];

foreach ( $value as &$v ) {
    $v = (int)$v;
}

var_dump( $value );

^ takes 64ms

Change 492579 merged by jenkins-bot:
[mediawiki/core@master] Use (int) rather than intval()

https://gerrit.wikimedia.org/r/492579

Anomie closed this task as Resolved.Feb 25 2019, 3:34 PM
Anomie assigned this task to Reedy.
Anomie added a subscriber: Anomie.

What do we do about the array_map( 'intval' calls? Something? Nothing?

Nothing. I see no reason to change to a slower method for some misguided avoidance of intval().

Partly unrelated, I would advice against array_map with lambdas, as it can be surprisingly slow. Using PHP 7.2.4, let:

$a = array_map('strval', range(1, 1000));

$s = microtime(true);
for ( $i=0;$i<=10000;$i++ ) {
  xxxxxxx
}
print_r(microtime(true)-$s);

Now, replacing xxxxxxx with array_map('intval',$a);: ~0.6 seconds
Replacing it with array_map( function( $v ) { return (int)$v; }, $a );: ~0.9 seconds
...And with the good ol' foreach: ~0.36 seconds

Now, replacing xxxxxxx with array_map('intval',$a);: ~0.6 seconds

On my laptop I get ~0.35s with PHP 7.0.31, ~0.36 with PHP 7.2.11, and ~0.32 with PHP 7.3.2.

Replacing it with array_map( function( $v ) { return (int)$v; }, $a );: ~0.9 seconds

~0.50 with 7.0.31, ~0.51 with 7.2.11, and ~0.43 with 7.3.2.

Assigning $cb = function( $v ) { return (int)$v; } at the start and making xxxxxxx be array_map( $cb, $a ) didn't seem to make a difference, BTW.

...And with the good ol' foreach: ~0.36 seconds

What exactly did you replace the xxxxxxx with there?

Using $x = []; foreach( $a as $k => $v ) { $x[$k] = (int)$v; } I get ~0.36 with 7.0.31, ~0.35 with 7.2.11, and ~0.34 with 7.3.2.

Using $x = $a; foreach( $x as &$v ) { $v = (int)$v; } unset( $v ); I get ~0.41 with 7.0.31, ~0.38 with 7.2.11, and ~0.38 with 7.3.2.

I do get significantly faster times if I screw it up by doing foreach( $a as $k => $v ) { (int)$v; } (i.e. not actually building the return value) or foreach( $a as &$v ) { $v = (int)$v; } unset( $v ); (i.e. only the first iteration of the outer loop actually casts).

Now, replacing xxxxxxx with array_map('intval',$a);: ~0.6 seconds

On my laptop I get ~0.35s with PHP 7.0.31, ~0.36 with PHP 7.2.11, and ~0.32 with PHP 7.3.2.

I did it online, so that may play a role... I forgot to mention that I repeated the test ~5 times per case and averaged the results.

Assigning $cb = function( $v ) { return (int)$v; } at the start and making xxxxxxx be array_map( $cb, $a ) didn't seem to make a difference, BTW.

I thought it would TBH.

...And with the good ol' foreach: ~0.36 seconds

What exactly did you replace the xxxxxxx with there?

A trivial non-mutable foreach, foreach( $a as $v ) { $v = (int)$v; }.

The timing differences may vary depending on several factors, of course. I recently faced a case where performing a cheap operation on a hashmap (adding a constant to all values) executed in ~1.5 seconds with array_map, and ~0.9 seconds with foreach. So I searched a bit on the web and did some testings, and found out that array_map (at least with lambdas) has poor performance compared to the equivalent foreach.

RobH mentioned this in Unknown Object (Task).May 14 2019, 7:48 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:38 PM