Repository navigation
Temporary red run for pull request 72, do not merge - #78
Closed
jwrosewell wants to merge 15 commits into
Closed
jwrosewell wants to merge 15 commits into
jwrosewell wants to merge 15 commits into
Conversation
PHP builds DeviceDetectionOnPremise, IpIntelligenceOnPremise and the pipeline on every request, while the native engine is built once, when PHP starts. These tests say that a request that reads a few values builds neither the table of every property in the data set nor the pipeline's database of them, that the table is the same when it is asked for, and that the database is built by the first search. CoreTests::testUpdateProperties now searches once before it changes the element's properties, because the database is taken at the first search rather than when the pipeline is built, and it still checks that updatePropertyList brings the change into the database.
PHP builds the pipeline on every request, and the Pipeline constructor built the database of every element's property metadata that getWhere searches, which with the TAC file took 1.6 ms of a 5.1 ms request. The database is now built when it is first read, so a request that never searches does not build it. Once built, updatePropertyDatabaseForFlowElement updates it as before.
PHP constructs DeviceDetectionOnPremise on every request, while the native engine is built once, when PHP starts. Each request read the metadata of every property in the data set (284 in the TAC file), which took 1.6 ms of a 5.1 ms request. The engine now builds the table of every property, and the index table, when they are first read, and a read looks up only its own property in native code, by the name the engine was built with. A read uses the table instead once something has built it, and a name the engine was not built with is looked up in the table, so every answer is the one the table gave. After a data refresh the table is built again from the new data set.
IpIntelligenceOnPremise is constructed on every request too, and read the metadata of every available property each time. It now builds the table and the index table when they are first read, and a read looks up only its own property, as Device Detection does. When metadata is needed after the engine is closed, it throws its engine closed message rather than failing.
With the table builds gone, a request through a pipeline that reads by name is faster than one that reads by index, so these tests say every value accessor is handed the property's name. The stub tests are those of the earlier by-name draft. The tests of the index table go, and a property in the data set that the engine was not built with now carries the native code's reason for having no value.
Timed over whole requests in one runner job, with the TAC file and the table builds gone, a request that reads by name is faster than one that reads by index at 5 and 15 names and level at 1. A read already finds each property's spelling among the names the engine was built with, so it hands that name to the native accessor, which looks it up itself, rather than also finding its index in PHP. The index table, getPropertyIndex, the not built message and the IP Intelligence start-up setting reader, which only the index table used, go. A property in the data set that the engine was not built with is read by name, so it carries the native code's reason for having no value.
The Device Detection test of a property the engine was not built with now checks that the table of every property was built, which only the fallback in findProperty does there, and that the no value message is not the one the missing property service writes. The IP Intelligence close test asks whether a property exists before and after the close.
Run 43176251578 showed the test asking about the name it had already asked about while the engine was open, which the engine keeps for the rest of the request, so nothing reached the engine and the test could not fail for the reason it was written for.
Looking a property up needs the engine, which close() releases, so __isset now answers false rather than letting the engine closed message out of isset, which cannot throw. The comment in each findProperty says that the fallback reads the table through __get, which the isset above it does not.
Run 43185229593 failed testCloseReleasesResultsAndEngine on the head, because the pipeline that test builds carries the JSON bundler and the set header elements, and each reads every property, which builds the table of every property. The table answers without the engine, so the read never reached the closed engine and isset answered true. The closed engine case moves to its own test, whose pipeline leaves those two elements out, and which asserts the table is still unbuilt so it cannot pass on the table alone. testCloseReleasesResultsAndEngine goes back to what it was, covering the default pipeline.
Throwaway branch. The head of pull request 72 with the catch in SwigData::__isset removed, to show that testAskingAClosedEngineAboutAPropertyAnswersFalse fails without it.
The catch in SwigData::__isset took every exception and answered false, so a fault reading the native metadata would have read as the engine having no property of that name. Only the engine closed message now answers false, and anything else reaches the caller. Two stub tests pin both halves, and they need no data file, so they run in every job.
# Conflicts: # ip-intelligence/src/SwigData.php
Merged
Contributor
Author
|
Closing. This was a temporary red run for #72 and was never meant to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throwaway. This branch is pull request 72's head with only the catch removed from SwigData::__isset, so the closed engine tests should fail here. Opened only to let the test suite run. Do not merge.