Network to 16 byte struct

[jashook]

First potential commit to the repo 👋. Network type was a 12 byte reference type, move it to a 16 byte value type to avoid unnecessary heap allocations. Change intentionally adds an unused 4 byte buffer field for natural alignment x64 and aarch64.

Also adds another benchmark to FindIsp including types from https://github.com/maxmind/GeoIP2-dotnet

Note, the benchmarks seem suspect. Based on reading through the logic there are more allocations expected than just the network class. As there are are dictionaries and other complex types. Will debug the benchmark tomorrow, worried the benchmark is not operating fully as expected.

Edit: IP was not being found in the demo db I had.

Benchmarks: osx m1

Baseline

Method	Mean	Error	StdDev	Gen0	Allocated
City	322.0 us	2.39 us	2.23 us	4.8828	31.25 KB
Isp	320.4 us	2.13 us	1.88 us	4.8828	31.25 KB

Change

Method	Mean	Error	StdDev	Allocated
City	318.0 μs	2.01 μs	1.88 μs	-
Isp	317.8 μs	2.69 μs	2.52 μs	1 B

[gemini-code-assist]

Summary of Changes

Hello @jashook, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the Network type from a class to a struct to optimize memory usage and performance. By converting it to a value type, it eliminates heap allocations, and an intentional padding field ensures optimal 16-byte alignment for improved efficiency on specific architectures.

Highlights

• Type Conversion: The Network type has been converted from a sealed class (reference type) to a struct (value type).
• Performance Optimization: This change aims to avoid unnecessary heap allocations, improving performance, especially on aarch64 and Linux x64.
• Memory Alignment: An unused 4-byte padding field (_Padding) was added to ensure the Network struct is naturally aligned to a 16-byte boundary.

Changelog

• MaxMind.Db/Network.cs
  • Changed Network from a sealed class to a struct.
  • Added a private _Padding integer field to ensure 16-byte alignment.

Activity

• This appears to be the initial contribution to the repository from the author, jashook.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the Network type from a class to a struct to reduce heap allocations and improve performance by ensuring it's a 16-byte value type. This is a sound optimization strategy. My feedback includes a suggestion to align the naming of the new padding field with the project's existing conventions and to improve the clarity of the associated comments.

[gemini-code-assist]

The naming of the new field _Padding does not follow the existing convention in the project for private fields, which is _camelCase (e.g., _database, _fileName in Reader.cs). It should be _padding for consistency.

Additionally, the comment can be made more concise and formal. The reference to 'linux x64' can be simplified to 'x64' as alignment benefits are typically architecture-specific, not OS-specific. The part about ignoring the compiler warning is also redundant given the #pragma directive.

// This field is intentionally unused. It pads the struct to 16 bytes
// to improve performance on aarch64 and x64.
#pragma warning disable CS0169
        private int _padding;
#pragma warning restore CS0169

[jashook]

Will keep linux x64 as the 16 byte type is treated specially in that ABI.

[jashook]

Will keep the comment open for context.

[horgh]

Thanks for the PR!

It looks like this would be a breaking change, so I think we'd be reluctant to go ahead with it unless there is strong evidence it's going to be a big benefit.

Has your benchmarking shown it to make an appreciable difference? It sounds like you didn't see what you expected.

Claude thinks the padding won't help:

Padding may not achieve the stated goal (Network.cs:18-22)

The comment says "Align the type on a 16 byte boundary." However:

• There's no [StructLayout(LayoutKind.Sequential)] attribute. For structs containing reference types (IPAddress), the CLR is permitted to use Auto layout at the JIT level and reorder fields, meaning the padding field may not actually end up where you expect.
• Even if the layout is sequential and the struct is 16 bytes, this controls the size, not the alignment. The CLR doesn't guarantee 16-byte alignment for arbitrary structs on the stack or in arrays.

It's also not sure we save any allocations:

Before (class)

1. new Network(ipAddress, prefixLength) in Reader.cs:247 — 1 heap allocation (class instance)
2. Passed as Network? through the method chain (just copying an 8-byte reference)
3. parameters[item.Position] = network in Decoder.cs:460 — no boxing, it's already a reference type on the heap

After (struct)

1. new Network(ipAddress, prefixLength) in Reader.cs:247 — 0 heap allocations (stack)
2. Passed as Network? through the method chain (Nullable, ~20 bytes copied by value at each call: Decode → DecodeByType → DecodeMap → DecodeMapToType → SetNetwork)
3. parameters[item.Position] = network in Decoder.cs:460 — 1 boxing allocation to store the struct into object?[]

So for the standard Find path, you're trading one class allocation for one boxing allocation. Net allocation reduction: zero. The object that lands on the heap is roughly the same size either way.

It's actually arguably slightly worse in the struct version because:

• Nullable is ~20 bytes (16-byte struct + bool + padding), copied by value through 5 method calls on the hot path, vs. copying an 8-byte reference before
• The 16-byte padding alignment benefit is lost once wrapped in Nullable (which is ~20 bytes, not a power of 2)