Repository navigation
Default IP Intelligence to the Balanced performance profile - #69
Draft
jwrosewell wants to merge 2 commits into
Draft
jwrosewell wants to merge 2 commits into
jwrosewell wants to merge 2 commits into
Conversation
The IP Intelligence on-premise specification gives Balanced as the default performance profile. The on-premise pipeline builder defaulted to MaxPerformance, and the engine builder applied no profile unless the caller set one, so it ran with whatever configuration the native ConfigIpi constructor starts from, which is the in-memory one in the C library this repository pins. Its setPerformanceProfile documentation and @DefaultValue already said Balanced. The engine builder now applies Balanced to its native configuration in a field initializer, before the constructor sets the concurrency, so a builder whose caller sets no profile is Balanced whatever the native library starts from. The pipeline builder holds no profile until the caller sets one and passes it on only then, because applying Balanced again would replace the concurrency the engine builder set. BuilderTests and the new OnPremisePerformanceProfileTests compare the collection settings each builder gives the engine with the Balanced and in-memory settings the native library defines.
…the engine The comment above the engine in MetadataOnPrem said the max performance profile while the code sets LowMemory, and gave the C# call for getting the engine from a pipeline. It now names the low memory profile and gives pipeline.getElement(IPIntelligenceOnPremiseEngine.class).
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.
Problem
The IP Intelligence on-premise specification gives Balanced as the default performance profile (ip-intelligence-on-premise.md, the Performance Profile row). Java gave something else from both of its builders.
IPIntelligenceOnPremisePipelineBuilderdefaulted to MaxPerformance, as its javadoc said (lines 64 and 65), and always passed that profile to the engine builder (line 390).IPIntelligenceOnPremiseEngineBuildercreatednew ConfigIpiSwig()and applied no profile unless the caller set one (line 67), althoughsetPerformanceProfilesays "Default value is Balanced" and carries@DefaultValue("Balanced")(lines 113 to 121). The C++ConfigIpi()constructor in the C library this repository pins starts from the in-memory configuration (51degrees-cxxsrc/ipi/ConfigIpi.cppline 30), so an engine builder left alone, including one a configuration file names without aPerformanceProfile, loaded the whole data file into memory.Change
IPIntelligenceOnPremiseEngineBuildernow starts its native configuration from the Balanced profile, in a field initializer (balancedConfig), so a builder whose caller sets no profile is Balanced whatever configuration the native library starts from, and@DefaultValue("Balanced")is now true. The initializer runs before the constructor'sconfig.setConcurrency(...)line, which is unchanged, because applying a profile replaces the concurrency of every collection (setPerformanceFromExistingConfigcopies the whole profile over the configuration).IPIntelligenceOnPremisePipelineBuildernow holds no profile until the caller sets one, and passes a profile to the engine builder only then, so the engine builder's Balanced default applies. Passing Balanced explicitly instead would have replaced the concurrency the engine builder set (the processor count) with the native default of 10.A caller who names a profile gets that profile, exactly as before. The C library is changing its own
ConfigIpi()starting point separately, and this change does not depend on that.Two examples build an engine with no profile and so move from the in-memory configuration to Balanced when they take up a release with this change, being the
UpdateDataFileconsole example (pipeline builder) and thegetting-started.apiweb example (engine builder).DataFileHelper.getDatafileMetaDatain the examples' shared module does the same to read the data file's details. None of them says which profile it uses.The comment above the engine in the
MetadataOnPremconsole example said the max performance profile, while the code sets LowMemory, and gave the C# call for getting the engine from a pipeline. A second commit makes the comment name the low memory profile and give the Java call,pipeline.getElement(IPIntelligenceOnPremiseEngine.class). The example's code, and its profile, are unchanged. I compiled the examples' console module and its shared module withmvn -B -pl console -am compileinexamples/ip-intelligence, and the build succeeded.Tests
BuilderTestsinip-intelligence.engine.on-premise.Builder_NoProfile_CollectionsAreBalancedreads the native configuration a new builder hands to its engine and checks that every collection's loaded count and cache capacity equal those of a configuration withsetBalanced()applied, and differ from one withsetMaxPerformance()applied.Builder_MaxPerformance_CollectionsAreInMemorychecks that naming MaxPerformance, as an enum value or by name as a configuration file does, still gives the in-memory settings. Neither needs a data file.OnPremisePerformanceProfileTestsinip-intelligence, new because the existingIPIntelligenceOnPremisePipelineBuilderTestis marked@Ignore.build_NoProfile_EngineIsGivenBalancedbuilds a pipeline with the profile and the concurrency left at their defaults, reads the native configuration the built engine holds, checks it against Balanced and InMemory as above, and checks that its concurrency is the one a new engine builder sets, so the pipeline did not apply a profile over it.build_LowMemory_EngineIsGivenLowMemorychecks that a profile the caller names still reaches the engine. Both need an IP Intelligence data file and are skipped, with a message saying how to get one, when there is none.The comparison takes the Balanced and InMemory values from the native library itself, by applying each profile to a fresh configuration, so it does not copy numbers from the C code and stays right if the C library changes them. Concurrency is left out of the collection comparison, because the engine builder sets its own.
What the tests prove is that the configuration each builder hands to the engine carries Balanced's collection settings when the caller names no profile. They do not measure memory, which the next section does once by hand.
Checks
The native library was built from the C library this repository pins (51degrees-cxx at e866f6d) with Visual Studio 2022 on Windows x64, as part of the Maven build, and the tests ran against it on Java 21 with the free Lite data file (
51Degrees-LiteV41.ipi, 478 MB) given through51DEGREES_IPI_PATH. From the repository root:BuilderTestsran 3 with 0 failures andOnPremisePerformanceProfileTestsran 2 with 0 failures and 0 skipped.Builder_NoProfile_CollectionsAreBalancedandbuild_NoProfile_EngineIsGivenBalancedfailed, each witharrays first differed at element [0]; expected:<1> but was:<0>, which is the strings collection configured as InMemory (nothing loaded at the start) where Balanced loads one entry. The two tests that name a profile passed.setBalanced()applied reported a concurrency of 10, against 12 processors on the machine, which is why the pipeline builder no longer applies Balanced itself.I also measured the process with a throwaway test, not committed, that built the pipeline with no profile and then with MaxPerformance, in separate JVMs with a fixed 768 MB heap, and looked up the same 20,000 random IPv4 addresses twice. Private bytes are the process's committed memory as Windows reports it, read after a garbage collection.
So on the Lite file the default engine used about 170 MB less, and a lookup took a few microseconds longer on the second pass, with the data file already in the operating system's file cache. The first pass was too noisy to compare. I did not measure the Enterprise data file.
For the release notes
The IP Intelligence on-premise engine now uses the Balanced performance profile when none is set, as the specification says, where the pipeline builder used MaxPerformance and the engine builder used the native library's in-memory configuration. Anyone relying on the default will use less memory after upgrading, because the data file is no longer loaded into memory as a whole (about 170 MB less with the free Lite data file), and lookups that read from the file will be slower. Naming
MaxPerformancewithsetPerformanceProfile, or<PerformanceProfile>MaxPerformance</PerformanceProfile>in a configuration file, keeps the old behaviour.What remains
ConfigIpi()starting point separately. This change does not rely on that, and keeps the engine builder's concurrency line as it is.