[jwrosewell]

Takes out the read path that #66 added, so the on-premise Device Detection and IP Intelligence engines read every value by the property's name again, as they did before #66.

Problem

#66 made both on-premise engines read every value by its index in the list of names the engine was built with, through a table from names to indexes. The table is built when DeviceDetectionOnPremise or IpIntelligenceOnPremise is constructed, and PHP constructs them on every request, so the table is built on every request. A read by index is faster than a read by name, but the table has to be built before the first read, and the question is whether a whole request is faster.

Measurement

One runner job, run 43064604148, built the Device Detection extension from cxx 7c4ac436 with PHP 8.3 and loaded the TAC file (109 MB). It timed two trees of PHP code against the same extension.

• By index, rc as it is now (d876e75, whose tree is the tree the job checked out).
• By name, the same tree with the one line that builds the table taken out of DeviceDetectionOnPremise, so the table stays empty and readKey hands every accessor the name, which is Read every on-premise value by its build-time property index #66's read path reverted.

A request builds DeviceDetectionOnPremise and a pipeline with no JavaScript or set-header elements, processes one User-Agent and reads each name once through $flowData->device->get. Each figure is 3,000 requests after 100 to warm up, in its own PHP process with required_properties set to the first 1, 5 or 15 of IsMobile, BrowserName, PlatformName, DeviceType, HardwareVendor, BrowserVersion, PlatformVersion, HardwareModel, HardwareName, ScreenPixelsWidth, ScreenPixelsHeight, IsTablet, IsSmartPhone, BrowserVendor and PlatformVendor. There were 10 rounds, and the order of the two trees was swapped each round. The two trees read the same values at every size. The scripts were ci/tmp-timing/request.php and ci/tmp-timing/run.py on a temporary branch, at 15f59f0d, which has since been deleted.

Medians of the 10 rounds, with the lowest and highest.

Names	By index (rc)	By name (#66's read path reverted)	Index minus name	Rounds where index was slower
1	5,583,272 ns (5,466,790 to 5,673,877)	5,538,868 ns (5,388,551 to 5,699,251)	+44,404 ns (+0.80%)	7 of 10
5	5,654,469 ns (5,484,297 to 5,790,857)	5,595,882 ns (5,504,714 to 5,736,703)	+58,587 ns (+1.05%)	7 of 10
15	5,615,328 ns (5,534,273 to 5,674,078)	5,578,554 ns (5,508,228 to 5,767,817)	+36,773 ns (+0.66%)	9 of 10

By index was slower at every size, and slower in 23 of the 30 rounds. The differences are smaller than the spread of each column, so the measurement shows that reading by index makes a whole request no faster, rather than putting a number on how much slower it is. The table alone took 1,345 to 2,563 ns for 15 names with the Lite file in run 43059409861 on the php#46 branch, and a read by index saved 24 to 70 ns there, so a request would need tens of reads before the time saved covered the table. A whole request took about 5.6 ms with the TAC file, so a difference of that size is lost in the spread of the rest of the request.

Specification

Finding Properties at 927ac677 says "Every read MUST use the position", and that the table "is built once, when the Engine is created". In PHP the Engine, the native engine of the extension, is created once when PHP starts, while the class that would hold the table is built on every request, so PHP cannot build the table once per Engine today. This draft is the faster approach, which James's rule says is used, and is a departure from that sentence of the specification for PHP until the table can be built once per process.

What changed

1. Tests first (128b439). device-detection/onpremise/tests/SwigDataIndexTests.php becomes SwigDataReadTests.php, whose stub tests say every value accessor is handed the property's name, a read through SwigData agrees with a native read by name, and a metric property is not read through a value accessor. In ip-intelligence/tests/SwigDataTests.php, testEveryReadGoesByName replaces the three index tests. PropertyIndexTests.php and the IP Intelligence tests of the table (testANameGivenTwiceKeepsItsFirstIndex and testASettingForTheRequestDoesNotTurnOnIndexes) go. testAnEngineBuiltWithNamesReadsTheSameValues stays, without its check of the table, so an engine built with names in an order of its own still reads the values an engine with no names reads.
2. The read path (98489b1). SwigData.php, Messages.php, IpIntelligenceOnPremise.php and the IP Intelligence StubSwigEngine.php are as they were before Read every on-premise value by its build-time property index #66 (90c3bdb~1), and none of them changed on rc after Read every on-premise value by its build-time property index #66. That takes out the table, getPropertyIndex, readKey, PROPERTY_NOT_BUILT and the IP Intelligence startupSetting. DeviceDetectionOnPremise.php loses the table, getPropertyIndex and buildPropertyIndexes, and keeps startupSetting, so Device Detection's restricted properties still come from required_properties as it was when PHP started, which Read every on-premise value by its build-time property index #66 also fixed and which is not about reading. The Device Detection stub engine loses its table.

Runs

• Red, the tests on 128b439 over the code before the change, run 43068724388. Ubuntu PHP 8.2 failed SwigDataReadTests::testEveryReadGoesByName and testMetricPropertiesAreNotReadThroughTheValueAccessors in Device Detection (2 of 53) and SwigDataTests::testEveryReadGoesByName in IP Intelligence (1 of 17), each because the accessors were handed 0, 1 and 2 where the names were expected, and the other jobs were cancelled when it failed. Its Setting 'PullRequestSha' line gives 128b439.
• Green and full build on the head 98489b1, run 43069475298. All 8 jobs passed, Ubuntu PHP 7.4, 8.0, 8.1, 8.2, 8.3, 8.4 and 8.5 and Windows PHP 8.3, and the two performance comparison jobs were skipped. Every one of the 9 Setting 'PullRequestSha' lines gives 98489b1, and Configure printed "Left out 5 macOS entries".

What remains

• Building the table once per process, for example in the native extension when the module starts, so that PHP can read by index without building the table on each request. Named for next week.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

[jwrosewell]

Closing unmerged. #72 supersedes this draft and is in rc.