Repository navigation
Read property values by index for an engine built with property names - #101
Merged
Merged
Conversation
C gives a property the place of its name among the names given, the first where the name is given more than once, and compares names without regard to case. The builder drops only an exact repeat, so IsMobile and ismobile both reach C. The engines' map keeps the last index for such a name, so the new tests fail on it. The test comments no longer say which test goes red or describe the change.
…he first Each engine now makes its map from the list of names its builder passes to the native engine, in the constructor, before any data is loaded. That is the list C takes the required property indexes from, one place for each entry in the order given, so the map holds the same indexes without running a detection on empty Evidence to get a results object, which for an engine built with names walked every component's graph at build. A name given more than once keeps its first place, as C keeps it. The map is final and never rebuilt, and the native engine's required properties configuration is no longer read back.
PropertyIndex_ReadByIndexAgainstByName builds the native engine with the same names, reads IsMobile 20,000 times by name and 20,000 times by index from the same native results, and 20,000 times through the typed accessor, in five rounds after a warm-up. It requires every read to give the same value and prints the lowest, median and highest time of one read, so each build reports the figures for its platform.
A read by name with no index goes through getValueAsBool(Swig.asBytes( name)), which is also how every read by name went on rc, so the timing test now reads by name that way rather than through the String overload.
…amed with The engine is built on the TAC file with a name the Lite file lacks, then reloaded from the Lite file. The name keeps its index, a read of it by index gives no value rather than an error, as a read by name does, and IsMobile still reads its own value. C maps a named index to the data set before it reads, and a name the data set lacks maps to -1, so the read reports no value. The test pins that, so it has no red.
jwrosewell
marked this pull request as ready for review
November 11, 2026 14:53
This was referenced Nov 11, 2026
Move cxx to the 51degrees-cxx release 4.5.105 and read by index on an engine built with no names
#82
Merged
Draft
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.
Both on-premise engines read a property value by its index when the engine was built with a list of property names, so each read skips the string lookup in the native layer, and the index keeps naming the same property after a reload.
The problem
James Rosewell, 10 November 2026: "The change for the index of the property is to return the index of the property in the list of properties provided at BUILD time and not for the data set. This is because when a reload happens other properties might be in different orders. The use of the index to avoid the string look up should be applied everywhere."
A property's index is its place in the list of names the engine was built with, and that list is fixed at build. 51degrees-cxx gives that index, at
src/common/properties.clines 478 to 506 at c6d4e4d9, the commit this repository'srcbuilds against. It sets "the required property index of each available property, which is the place of its name among the names given", keeps the first place where a name is given more than once (if (found->requiredPropertyIndex < 0)), and works the mapping out again for each data set it loads, so index i names the ith name given whatever order the data set holds. Names are compared without regard to case. A name the reloaded data set lacks keeps its index and gives no value.On
rc,DeviceDataHashDefaultandIPIntelligenceDataHashDefaultread every value by name, so each read pays a string lookup in the native layer, and nothing in Java holds the build-time order.What changes
Head 3553c50, which is 52bcfd1 with
rcmerged in at 1c2c578 and then at ddb7d04. Links are at 52bcfd1, whose files the merges do not change, because they bring in only #98's data update service change, #80's web integration change and #99's cloud request change, none of which touches the on-premise engines.DeviceDetectionHashEngineandIPIntelligenceOnPremiseEngineeach make one case-insensitive map from the names the engine was built with to their places in that list, in the constructor, before any data is loaded. The map is final and never rebuilt, so a reload cannot change it. A name given more than once keeps its first place, as C keeps it.RequiredPropertiesConfigSwigon the line before (Device Detection lines 336 to 344, IP Intelligence lines 252 to 260). That vector becomes C's list one entry for each name, in order, with nothing split, trimmed or skipped (copyNamesFromArray, lines 254 to 275), so the place of a name in the list is its index in C. Nothing is read from the data set, and no detection is run to build the map. The earlier version of this draft ran a detection on empty Evidence at build to get a results object to ask for names, which for an engine built with names walked every component's graph.RequiredPropertiesConfig::getProperties(). That method isconstbut, for a list given as one comma-separated string, runsstrtokover the string in place (RequiredPropertiesConfig.cpplines 91 to 108), so a second call returns only the first name and two threads cannot call it at once. Java builds the configuration from a vector, which takes the other branch, but the builder's list needs no native call at all.DeviceDataHashDefaultandIPIntelligenceDataHashDefaultlooks the name up in that map and calls the index overload of the same getter, whichsrc/common/ResultsBase.ialready exposes and the generated Java wrapper already carries. Typed accessors go throughAspectDataBase.getAstoDeviceDataBaseOnPremise.tryGetValue, which calls the samegetValueAs...methods, so they are covered by the same change.rc. That covers a property the engine was not built with, which already gives the missing property answer, and an engine built with no names, whose map is empty.An engine built with no names is not changed here
Giving such an engine every property name in the data set at build was measured on this repository, in run 42902030477, at about 70 percent slower processing with MaxPerformance and three to seven times slower with LowMemory for a read of one property, so it is not done. The C layer is instead to keep the first load's property names, in their order, across every reload, in 51degrees-cxx#148, which merged into 51degrees-cxx
rcat 09:36 UTC today as 1a92d51, 85 commits after the c6d4e4d9 this repository builds against. Reads on a no-names engine therefore stay by name here, and follow once this repository takes that change.A name the reloaded data set lacks
Every name the engine was built with has an index, whether or not the data set loaded at the time has the property. A read by index of such a name gives no value, as a read by name does, and not an error. C++ reads by index through
valueAsBoolAtand its neighbours (ResultsBase.cpplines 151 to 185), which ask whether the property has values before reading any. For an engine built with names,PropertiesGetAvailableIndexgives -1 for "a name the data set does not have", andresultsHashGetHasValuesanswers false for an available index below 0, so the read reports no value with the invalid property reason. A read by name of the same property goes throughgetRequiredPropertyIndex, finds no index, -1, and takes the same path. C's values function does set an out of range error for such a property (hash.clines 5834 to 5836), which is the error 51degrees-go#96 guards against, but the C++ layer this repository uses asks whether there are values first and never calls it for such a property. In Java, a read throughgetor a typed getter does not reach the index at all for such a name, becausetryGetValueaskspropertyIsAvailable, which looks the name up in the data set, first.getValues(String)onDeviceDataHashDefaultreads by index with no such check, so the test reads through it.PropertyIndex_NameTheReloadedFileLacksbuilds an engine on the TAC file with a name the Lite file lacks, found by comparing the two files' property lists, and IsMobile, reloads it from the Lite file, and requires the name to keep index 0,getValuesof it to give no value with a message and no error, and IsMobile still to read true. The test pins how C behaves, so it has no red run. IP Intelligence reads through the sameResultsBasemethods, which ask whether there are values first, but its own C below them is not checked here and no shrinking reload is tested for it.Why no test of the reload rule can go red
C already gives an engine built with names the place of each name in the build-time list, and works it out again on each reload, so a read by name was already correct after a reload, and reading by index gives the same value. The tests of the rule therefore pass on
rcas well, apart from needinggetPropertyIndex, which this draft adds. The change for the rule is about cost, being the string lookup each read no longer pays. The earlier red run, 42933171724, was a compile failure for that reason and shows nothing about behaviour.The name given twice is the one behaviour a test can show wrong. The builder drops only an exact repeat, so
IsMobileandismobileboth reach C, which gives that property index 0. The earlier version of this draft kept the last place, 2.The cost
A read by name against a read by index, through the native engine
PropertyIndex_ReadByIndexAgainstByNamebuilds the native engine with the same four names from the Lite file, processes one iPhone User-Agent, and reads IsMobile 20,000 times by name, throughgetValueAsBool(Swig.asBytes("IsMobile")), the call every read by name made onrc, and 20,000 times by index, throughgetValueAsBool(int), from the same native results. It also reads IsMobile 20,000 times through the typed accessor, which is the whole path a caller takes and now goes by index. It does five rounds after a warm-up, requires every read to give the same value, and prints the lowest, median and highest time of one read. It runs in every job of the full build.In the full build, run 43021207790 on the head, the medians of one read, with the lowest and highest of the five rounds in brackets, were as follows.
The median read by index was faster than the median read by name in eight of the nine jobs, by 9 to 50 percent. On Ubuntu Java 21 the median read by index was slower, 406 ns against 370 ns, although its fastest round, 272 ns, was faster than any round read by name, 361 ns at best. The spreads inside each job are wide, so one job's medians can cross. The three temporary runs on Ubuntu Java 17, in the table under How it was checked, each had the median read by index faster, for example 170 ns against 314 ns through the bytes overload.
The whole process, before and after
Run 42995350653 built this branch at 691ce2e, which has the change to the engines in full, and
rcc77fb05 side by side on one ubuntu-latest runner with Java 17, and timed each in turn for each data file and profile, over 5,000 User-Agents from20000 User Agents.csv. Each round times every measure in turn, and five rounds follow two warm-up rounds. The figures are the median nanoseconds for each User-Agent, or milliseconds for a build, with the lowest and highest of the five rounds in brackets. The checksum of every value read is the same in both trees, so they read the same values.Engine built with the 4 names
Engine built with no names
These figures cannot show whether the change makes a process faster or slower. Processing alone reads no property and the change does not touch it, yet it moved between 3.5 percent faster and 9.1 percent slower for the named engine. The engine built with no names, whose path the change leaves alone apart from an empty map lookup, moved between 43 percent faster and 6 percent slower, with a narrow spread inside each tree. That is the difference between two JVMs and two builds on one runner, and it is larger than anything one string lookup can add or save, because a detection takes 5 to 30 microseconds and the native read above saves well under one. Reading all four names was 2.0 to 6.1 percent faster in three of the four cases and 0.9 percent slower on TAC LowMemory. Build time for the named engine moved by half a millisecond or less in every case, so taking the names from the builder's list costs nothing measurable.
Two earlier runs are not used. Run 42983051435 printed each line cut short, and run 42986828586 timed each measure in turn, so the first measure in each JVM ran before the JVM had warmed up and processing alone came out slower than processing and a read.
How it was checked
PropertyIndexTestsis new in both on-premise modules. Device Detection has nine tests and IP Intelligence five.Each run used a temporary workflow on a
tmp/branch that builds the reactor with the native libraries on Ubuntu with Java 17, fetches the data files and runsPropertyIndexTestsin both modules.PropertyIndex_NameGivenTwiceHasFirstIndex"expected:<0> but was:<2>", 1 of 7 in Device Detection and 1 of 5 in IP IntelligenceStringoverload, 164 ns against 117 ns by index, and the second through the bytes overload a read by name really uses, 319 ns against 171 ns, both medians on Ubuntu Java 17rcmergesPropertyIndex_NameTheReloadedFileLacksamong them. A read by name took 314 ns against 170 ns by index, medians on Ubuntu Java 17The full build's 9 jobs without macOS all passed, being Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11, 17 and 21. Every
Setting 'PullRequestSha'line, in Configure and in each of the 9 jobs, is 3553c50, the head, and Configure printed "Left out 6 macOS entries". Each job printed the timing line above.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.
The branch has
rcmerged in three times, at c77fb05, which movescxxto c6d4e4d9, at 1c2c578, which carries #98, and at ddb7d04, which carries #80 and #99.Other open drafts
git merge-tree --write-treeof this head against every other open draft's head finds no conflict, apart from #43, #63, #67, #82 and #97, which conflict withrcitself, #82's being thecxxsubmodule. #98, #99 and #80 are merged intorcand are in this branch.What remains
getor a typed getter still askscontainsPropertyby name before it reads, so it now pays one name lookup where it paid two. Checking that by index as well would save the other, and is not done here.Version
Patch only.