Import global classes but fully qualify functions and constants

[paulbalandan]

Description
An alternative approach to #4890, which I am using in my libraries (except for the import part).

Strategy:

• All native classes are imported with use statements.
• A select subset of constants are fully qualified DIRECTORY_SEPARATOR, PHP_VERSION_ID, PHP_SAPI, PHP_INT_SIZE as frequently used. (This one is debatable)
• Native functions which are optimized by the Zend engine are fully qualified. See https://github.com/php/php-src/blob/php-7.2.6/Zend/zend_compile.c and https://github.com/php/php-src/blob/php-7.2.6/ext/opcache/Optimizer/pass1_5.c

Checklist:

• Securely signed commits

[MGatner]

I need @lonnieezell to weigh in. I'm too torn - I hate the preceding slashes but I know it is a performance gain.

[lonnieezell]

@paulbalandan Are you able to run any benchmarks comparing this branch with the optimizations to a version of the framework without? I'm very curious what the gain actually is.

I agree with @MGatner that I'm not a huge fan of the slashes. This feels almost like a premature optimization, but I could be off base. CodeIgniter isn't exactly slow right now, with the possible exception of the database layer. I feel there is probably room for optimizations there.

[paulbalandan]

I'm not sure how to proceed with benchmarking. Never did that before. Do you know of a tool or reference I can try out?

[paulbalandan]

Ok. Found a way to bench this with ApacheBench. Will send the results later.

[lonnieezell]

Oh, glad you found a way to test! I haven't really done any testing large scale either. I'm very curious to see what the results are.

[paulbalandan]

OK, here are the results with Apache Bench ab.

Some caveats with the benchmarks:

• I'm using my dev laptop instead of a dedicated server so results may not be representative. Maybe someone can try this in a server to have a better picture.
• My laptop is pretty low on resources. Maybe that's why the requests per second results are less than 1 each. Maybe needing a fresh server to run this.
• Maybe a better way to benchmark this is what Taylor Otwell is suggesting (to bench Laravel, Symfony and Zend): ab -t 10 -c 10 http://localhost:8080/ (50,000 requests at 10 concurrent at a time over a time limit of 10 seconds).
• I'm not an expert in interpreting the results of ab. I consulted SO for this.

Environment

OS: Windows 10 Home Single Language 19043.1110
PHP: 8.0.8 TS
Opcache enabled: Yes (both opcache.enable=1 and opcache.enable_cli=1)
Apache: Apache/2.4.48 (Win64) OpenSSL/1.1.1k PHP/8.0.8

Steps:

1. Fetch the latest develop branch and this PR branch in your local copy.
2. We'll be running the ab tool 5 rounds for each branch and summarize the results.
3. A resting period of 1 minute after each round will be followed to allow the server to not be overloaded.
4. We'll be using php spark serve using the URL http://localhost:8080/.
5. When switching branches, make sure to close the connection with the dev server with Ctrl+C. Delete also the files generated by opcache.

Metrics used

A: Requests per second (#/sec)
B: Time per request (mean) - measure of delay of single request
C: Time per request (mean, across all concurrent requests) - measure of performance

Results

The following are the results of running ab -n 100 -c 10 http://localhost:8080/
100 requests running 10 concurrently at a time

No.	Metrics	Develop	This branch
1	A B C	0.71 #/sec 13,997.467 ms 1,399.747 ms	0.72 #/sec 13,896.418 ms 1,389.642 ms
2	A B C	0.73 #/sec 13,666.197 ms 1,366.620 ms	0.73 #/sec 13,646.384 ms 1,364.638 ms
3	A B C	0.73 #/sec 13,606.357 ms 1,360.636 ms	0.73 #/sec 13,642.761 ms 1,364.276 ms
4	A B C	0.74 #/sec 13,519.923 ms 1,351.992 ms	0.74 #/sec 13,560.967 ms 1,356.097 ms
5	A B C	0.73 #/sec 13,776.302 ms 1,377.630 ms	0.73 #/sec 13,749.684 ms 1,374.968 ms
Average	A B C	0.728 #/sec 13,713,249 ms 1,371.325 ms	0.73 #/sec 13,699.243 ms 1,369.924 ms

Results using ab -t 10 -c 10 http://localhost:8080/ (one round each only)

No.	Metrics	Develop	This branch
1	A B C	0.74 #/sec 13,534.104 ms 1,353.410 ms	0.75 #/sec 13,421.327 ms 1,342.133 ms

[MGatner]

Wow this is awesome! Thanks @paulbalandan. I can run on a dedicated server if anyone is suspicious of laptop results, but to me this seems to show: a) there is a noticeable performance difference, and b) it isn't large enough to dictate our style choices.

I'm still not opposed to applying this but I think in the realm of optimization there are a lot of other changes we could make for higher gains.

[MGatner]

I ran this on a dedicated server but my results never stabilized. I followed our process, only thing I didn't know how to do was "Delete also the files generated by opcache."

Attempt 1

Time taken for tests:   1.908 seconds
Complete requests:      100
Failed requests:        0
Total transferred:      1843500 bytes
HTML transferred:       1822700 bytes
Requests per second:    52.40 [#/sec] (mean)
Time per request:       190.849 [ms] (mean)
Time per request:       19.085 [ms] (mean, across all concurrent requests)
Transfer rate:          943.31 [Kbytes/sec] received

Attempt 2

Time taken for tests:   1.956 seconds
Complete requests:      100
Failed requests:        1
   (Connect: 0, Receive: 0, Length: 1, Exceptions: 0)
Total transferred:      1843499 bytes
HTML transferred:       1822699 bytes
Requests per second:    51.13 [#/sec] (mean)
Time per request:       195.569 [ms] (mean)
Time per request:       19.557 [ms] (mean, across all concurrent requests)
Transfer rate:          920.54 [Kbytes/sec] received

Attempt 3

Time taken for tests:   2.254 seconds
Complete requests:      100
Failed requests:        98
   (Connect: 0, Receive: 0, Length: 98, Exceptions: 0)
Total transferred:      1843487 bytes
HTML transferred:       1822687 bytes
Requests per second:    44.36 [#/sec] (mean)
Time per request:       225.408 [ms] (mean)
Time per request:       22.541 [ms] (mean, across all concurrent requests)
Transfer rate:          798.68 [Kbytes/sec] received

Attempt 4

Time taken for tests:   2.082 seconds
Complete requests:      100
Failed requests:        6
   (Connect: 0, Receive: 0, Length: 6, Exceptions: 0)
Total transferred:      1843494 bytes
HTML transferred:       1822694 bytes
Requests per second:    48.03 [#/sec] (mean)
Time per request:       208.189 [ms] (mean)
Time per request:       20.819 [ms] (mean, across all concurrent requests)
Transfer rate:          864.74 [Kbytes/sec] received

Attempt 5

Time taken for tests:   2.293 seconds
Complete requests:      100
Failed requests:        4
   (Connect: 0, Receive: 0, Length: 4, Exceptions: 0)
Total transferred:      1843496 bytes
HTML transferred:       1822696 bytes
Requests per second:    43.60 [#/sec] (mean)
Time per request:       229.336 [ms] (mean)
Time per request:       22.934 [ms] (mean, across all concurrent requests)
Transfer rate:          785.00 [Kbytes/sec] received

Attempt 6

Time taken for tests:   1.985 seconds
Complete requests:      100
Failed requests:        0
Total transferred:      1843500 bytes
HTML transferred:       1822700 bytes
Requests per second:    50.37 [#/sec] (mean)
Time per request:       198.527 [ms] (mean)
Time per request:       19.853 [ms] (mean, across all concurrent requests)
Transfer rate:          906.83 [Kbytes/sec] received

[lonnieezell]

@MGatner which of these is a baseline without the updates? Or are all of these the optimized branch?

[MGatner]

Those are all on develop! Since they didn't seem to be trending towards any consistency I never ran the optimized branch. Still hoping @paulbalandan has some input on improving my methodology. Alternatively I'm working with PHPBench for a code-side benchmark system - still probably a bit off, time has been scarce with summer vacation drawing to an end.

[paulbalandan]

Since you're using a server already, try using a live address instead of localhost (via php spark serve).

The "Delete files generated by opcache" is found in this ini setting opcache.file_cache="C:/tmp" (in my case)

[MGatner]

I tried this again, using the live server instead of spark. Numbers were still wildly variable so I switched to the other method you mentioned (ab -t 10 -c 10) with opcache off and they normalized a little bit (ab still considers them "length failures", too inconsistent). With this setup the fqcn-symbols is showing almost half the requests per second, so something must still be wrong because I cannot imagine it is a) less efficient, and b) differing by that much.

[MGatner]

develop

Example

Server Software:        Apache/2.4.46
Server Hostname:        bench.gatner.be
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-RSA-CHACHA20-POLY1305,2048,256
Server Temp Key:        X25519 253 bits
TLS Server Name:        bench.gatner.be

Document Path:          /
Document Length:        18226 bytes

Concurrency Level:      10
Time taken for tests:   10.001 seconds
Complete requests:      1491
Failed requests:        1417
   (Connect: 0, Receive: 0, Length: 1417, Exceptions: 0)
Total transferred:      27498429 bytes
HTML transferred:       27176373 bytes
Requests per second:    149.09 [#/sec] (mean)
Time per request:       67.075 [ms] (mean)
Time per request:       6.707 [ms] (mean, across all concurrent requests)
Transfer rate:          2685.17 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        3   15  30.7      7    1079
Processing:     4   52  42.4     43     357
Waiting:        4   49  41.7     40     356
Total:          7   67  50.7     63    1116

Percentage of the requests served within a certain time (ms)
  50%     63
  66%     81
  75%     91
  80%     99
  90%    117
  95%    134
  98%    152
  99%    196
 100%   1116 (longest request)

Results

Requests per second: 149.09 [#/sec] (mean)
Time per request: 67.075 [ms] (mean)

Requests per second: 133.98 [#/sec] (mean)
Time per request: 74.638 [ms] (mean)

Requests per second: 139.06 [#/sec] (mean)
Time per request: 71.912 [ms] (mean)

Requests per second: 137.40 [#/sec] (mean)
Time per request: 72.781 [ms] (mean)

Requests per second: 149.17 [#/sec] (mean)
Time per request: 67.036 [ms] (mean)

[MGatner]

fqcn-symbols

Example

Server Software:        Apache/2.4.46
Server Hostname:        bench.gatner.be
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-RSA-CHACHA20-POLY1305,2048,256
Server Temp Key:        X25519 253 bits
TLS Server Name:        bench.gatner.be

Document Path:          /
Document Length:        18227 bytes

Concurrency Level:      10
Time taken for tests:   10.026 seconds
Complete requests:      757
Failed requests:        102
   (Connect: 0, Receive: 0, Length: 102, Exceptions: 0)
Total transferred:      13961238 bytes
HTML transferred:       13797726 bytes
Requests per second:    75.50 [#/sec] (mean)
Time per request:       132.450 [ms] (mean)
Time per request:       13.245 [ms] (mean, across all concurrent requests)
Transfer rate:          1359.81 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        3   11  15.3      4     133
Processing:    15  120  31.0    117     275
Waiting:       12  117  30.7    114     272
Total:         19  131  29.6    130     283

Percentage of the requests served within a certain time (ms)
  50%    129
  66%    140
  75%    148
  80%    152
  90%    163
  95%    176
  98%    197
  99%    233
 100%    283 (longest request)

Results

Requests per second: 75.50 [#/sec] (mean)
Time per request: 132.450 [ms] (mean)

Requests per second: 75.29 [#/sec] (mean)
Time per request: 132.824 [ms] (mean)

Requests per second: 76.47 [#/sec] (mean)
Time per request: 130.771 [ms] (mean)

Requests per second: 75.00 [#/sec] (mean)
Time per request: 133.334 [ms] (mean)

Requests per second: 76.05 [#/sec] (mean)
Time per request: 131.499 [ms] (mean)

[lonnieezell]

Weird. I would not have expected those results. I actually expected there not to be much change but the new stuff is 1/2 as fast?

[MGatner]

Like I said I can only assume that something is wrong. These are tools I am not familiar with but that cannot possibly be accurate!

[MGatner]

My benchmarking solution isn't ready to unveil yet but I did run these two branch through what I have so far. The proposed fqcn-symbols branch once again does very poorly:

+--------------+----------+-----+------+-----+----------------+----------------+-----------------+
| benchmark    | subject  | set | revs | its | mem_peak       | mode           | rstdev          |
+--------------+----------+-----+------+-----+----------------+----------------+-----------------+
| NewsBench    | benchRun |     | 100  | 5   | 8.165mb -1.14% | 1.844ms -7.98% | ±2.00% +245.66% |
| WelcomeBench | benchRun |     | 30   | 10  | 6.787mb -0.94% | 1.416ms -8.42% | ±1.14% +149.56% |
+--------------+----------+-----+------+-----+----------------+----------------+-----------------+

So maybe there is something else going on here?

EDIT: Nevermind, I was reading it wrong! The deviation is much worse, but the average execution time improves ~8%, which seems more ballpark like what we are after.

[lonnieezell]

Yeah, I'm not really sure what's causing the numbers - but I don't think this one should hold up a release. Not sure what else is needed before the next release, though, honestly.

[paulbalandan]

ab is fairly new to me so I honestly cannot tell what's wrong with the figures. But, I agree this should not be a blocker for a release.

[MGatner]

If that's the case, here is my proposal:

1. Leave the two FQCN PRs in place for now
2. Create codeigniter/coding-standard and move the CS Fixer work Paul has done over to there
3. Reimplement codeigniter/coding-standard as a dev dependency here and other projects (cache, website, etc)
4. Release 4.1.4 "Style change", rebase and roll 4.2 into develop
5. Rebase these PRs to await my forthcoming benchmark automation

[paulbalandan]

I'm amenable to the proposal. My concern only is on point 2. The name is currently taken by the CS based on codesniffer, so I'm thinking codeigniter4/revised-coding-standard or other else more suitable??

[MGatner]

I'm proposing we use the newly-acquired organization "codeigniter/" (no "4"). Since this will ultimately be our style rules for all CodeIgniter projects, and the eventual goal is to host them out of that organization, this seems like a good first candidate. @lonnieezell ?

[lonnieezell]

I'm proposing we use the newly-acquired organization "codeigniter/" (no "4"). Since this will ultimately be our style rules for all CodeIgniter projects, and the eventual goal is to host them out of that organization, this seems like a good first candidate. @lonnieezell ?

I am fine with this. As we had discussed elsewhere moving everything to that org come version 5, anyway. I don't see a need for a new set of style rules then so this is a perfect first candidate. :)

[MGatner]

Sounds good! Yes normally I would wait for version 5 but because of the conflict Paul mentioned I think this is best.

@lonnieezell can you make sure the team has organization access to GitHub.com/codeigniter? Or at least just me for now so I can create the repo?

[lonnieezell]

@MGatner I think everyone has been invited now.

[paulbalandan]

Alright, on to it.

[paulbalandan]

Closing this for now. The cost of resolving merge conflicts and maintaining PR up to date with latest develop is way too much than its perceived benefits. I can just recreate this when the time comes.