feat: integrate caching into network_process objects #87

wiresock · 2025-09-03T08:33:18Z

Refactor proxy port resolution caching to store results directly in network_process objects instead of maintaining separate cache structures. This eliminates cache invalidation complexity and improves performance by leveraging natural data locality.

Fixes #86

Refactor proxy port resolution caching to store results directly in network_process objects instead of maintaining separate cache structures. This eliminates cache invalidation complexity and improves performance by leveraging natural data locality. Fixes #86

gemini-code-assist

Summary of Changes

Hello @wiresock, 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 significantly refactors the proxy port resolution mechanism by moving caching responsibilities from a centralized socks_local_router cache to individual network_process objects. This change aims to improve performance through better data locality and reduce complexity by removing explicit cache invalidation. The network_process structure has been extended to hold proxy-related state, and the logging infrastructure has been enhanced with new, more specific macros.

Highlights

Integrated Caching: Proxy port resolution caching is now directly integrated into network_process objects, eliminating separate cache structures previously managed by socks_local_router.
Simplified Cache Management: The previous complex cache invalidation logic, including explicit clear_match_cache() and cleanup_match_cache() calls, has been removed, simplifying the codebase.
Enhanced Process Information: The network_process struct now includes new fields: tcp_proxy_port, udp_proxy_port, excluded, bypass_tcp, and bypass_udp, to store caching and bypass decisions directly within the process object.
Refactored Proxy Mapping Storage: The name_to_proxy_ map in socks_local_router has been replaced with a std::multimap<size_t, std::wstring> named proxy_to_names_ for more flexible storage of proxy index to process pattern mappings.
Improved Logging Macros: New convenience logging macros (NETLIB_ERROR, NETLIB_WARNING, NETLIB_INFO, NETLIB_DEBUG, and their pointer/string variants) have been introduced for clearer and more concise logging, and the default global log verbosity has been adjusted.

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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

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 issue 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.

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. ↩

Copilot

Pull Request Overview

This PR refactors the proxy port resolution caching mechanism by eliminating the external cache data structures and instead storing proxy-related information directly within the network_process objects. This approach reduces complexity by removing cache invalidation logic and improves performance through better data locality.

Key changes include:

Replaced external match_cache_ with direct caching in network_process objects
Changed name_to_proxy_ mapping to proxy_to_names_ multimap for better iteration
Added proxy port and status fields to network_process structure

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
netlib/src/proxy/socks_local_router.h	Removes external cache structures, updates data mapping, and implements direct object caching
netlib/src/log/log.h	Changes default log verbosity and adds convenience logging macros
netlib/src/iphelper/process_lookup.h	Adds proxy-related fields to network_process and updates logging calls

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-03T08:35:02Z

netlib/src/proxy/socks_local_router.h

+            if (process->excluded || process->bypass_udp)
+                return packet_filter::packet_action{ packet_filter::packet_action::action_type::pass };
+
+            if (const auto port = process->udp_proxy_port? process->udp_proxy_port:get_proxy_port_udp(process); port.has_value())


The ternary operator usage is unclear due to missing space after the '?' operator. Consider using parentheses or splitting into separate statements for better readability.

Suggested change

if (const auto port = process->udp_proxy_port? process->udp_proxy_port:get_proxy_port_udp(process); port.has_value())

if (const auto port = (process->udp_proxy_port ? process->udp_proxy_port : get_proxy_port_udp(process)); port.has_value())

Copilot · 2025-09-03T08:35:02Z

netlib/src/proxy/socks_local_router.h

+            if (process->excluded || process->bypass_tcp)
+                return packet_filter::packet_action{ packet_filter::packet_action::action_type::pass };
+
+            if (const auto port = process->tcp_proxy_port? process->tcp_proxy_port:get_proxy_port_tcp(process); port.has_value())


The ternary operator usage is unclear due to missing space after the '?' operator. Consider using parentheses or splitting into separate statements for better readability.

Suggested change

if (const auto port = process->tcp_proxy_port? process->tcp_proxy_port:get_proxy_port_tcp(process); port.has_value())

if (const auto port = (process->tcp_proxy_port ? process->tcp_proxy_port : get_proxy_port_tcp(process)); port.has_value())

Copilot · 2025-09-03T08:35:03Z

netlib/src/iphelper/process_lookup.h

+        std::optional<uint16_t> tcp_proxy_port = std::nullopt; // Optional tcp proxy port if the process is associated with a proxy
+        std::optional<uint16_t> udp_proxy_port = std::nullopt; // Optional udp proxy port if the process is associated with a proxy


The comment should start with a capital letter: 'Optional TCP proxy port' and 'Optional UDP proxy port'.

Suggested change

std::optional<uint16_t> tcp_proxy_port = std::nullopt; // Optional tcp proxy port if the process is associated with a proxy

std::optional<uint16_t> udp_proxy_port = std::nullopt; // Optional udp proxy port if the process is associated with a proxy

std::optional<uint16_t> tcp_proxy_port = std::nullopt; // Optional TCP proxy port if the process is associated with a proxy

std::optional<uint16_t> udp_proxy_port = std::nullopt; // Optional UDP proxy port if the process is associated with a proxy

Copilot · 2025-09-03T08:35:03Z

netlib/src/proxy/socks_local_router.h


-            // Iterate through each pair in the process to proxy mapping.
-            for (auto& [name, proxy_id] : name_to_proxy_)
+            // Cache miss - search through patterns


This comment is misleading as there is no cache miss in this context since the external cache was removed. The comment should be updated to reflect the actual operation being performed.

Suggested change

// Cache miss - search through patterns

// Search for a matching process pattern

gemini-code-assist

Code Review

This pull request refactors the proxy port resolution caching by integrating it into the network_process objects, which is a great simplification. It also introduces convenient logging macros.

My review focuses on a few key areas:

Caching Logic: I found a critical issue where the resolved proxy ports are not being cached correctly, which undermines the main goal of this refactoring.
Code Duplication: There's an opportunity to reduce code duplication between the TCP and UDP port lookup functions.
Logging Configuration: A default configuration change in the logging system might have unintended side effects and is worth noting.

Overall, the direction is good, but the caching implementation needs to be completed to be effective.

gemini-code-assist · 2025-09-03T08:35:44Z

netlib/src/log/log.h

     * @note Default value includes all components (timestamp, thread, logger, level, path)
     */
-    inline std::atomic<log_verbosity> global_log_verbosity{ log_verbosity::all };
+    inline std::atomic global_log_verbosity{ log_verbosity::none };


Changing the default global_log_verbosity from log_verbosity::all to log_verbosity::none is a significant behavioral change. This will disable all logging by default across the library. While this can be useful for production builds to reduce noise, it might be surprising for developers during debugging. Please ensure this change is intentional and well-documented in the project's release notes or documentation.

gemini-code-assist · 2025-09-03T08:35:44Z

netlib/src/proxy/socks_local_router.h

        std::optional<uint16_t> get_proxy_port_udp(const std::shared_ptr<iphelper::network_process>& process)
        {
-            // Locks the proxy servers and process to proxy map for reading.
+            if (!process) return {};
+
            std::shared_lock lock(lock_);

-            // Iterate through each pair in the process to proxy mapping.
-            for (auto& [name, proxy_id] : name_to_proxy_)
+            // Cache miss - search through patterns
+            for (const auto& [proxy_id, process_pattern] : proxy_to_names_)
            {
-                // Check if the current process name contains the given process name.
-                if (match_app_name(name, process))
-                    // If it does, return the UDP proxy port associated with the proxy ID.
+                if (match_app_name(process_pattern, process))
+                {
                    return proxy_servers_[proxy_id].second
-                               ? std::optional(proxy_servers_[proxy_id].second->proxy_port())
-                               : std::nullopt;
+                        ? std::optional(proxy_servers_[proxy_id].second->proxy_port())
+                        : std::nullopt;
+                }
            }

-            // If the process name is not found in any of the keys, return an empty std::optional.
            return {};
        }


This function get_proxy_port_udp is nearly identical to get_proxy_port_tcp. The only difference is accessing .second instead of .first from the proxy_servers_ pair. This code duplication could be reduced by introducing a private helper function.

Consider refactoring this into a template or a function that accepts a member pointer/lambda to select the correct proxy server (.first or .second). This would improve maintainability by keeping the core logic in one place.

Copilot AI review requested due to automatic review settings September 3, 2025 08:33

gemini-code-assist bot reviewed Sep 3, 2025

View reviewed changes

Copilot AI reviewed Sep 3, 2025

View reviewed changes

gemini-code-assist bot reviewed Sep 3, 2025

View reviewed changes

fix: gemini-code-assist

63f5d50

wiresock merged commit 0e641da into main Sep 3, 2025

wiresock deleted the issue86 branch September 3, 2025 09:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: integrate caching into network_process objects #87

feat: integrate caching into network_process objects #87

Uh oh!

wiresock commented Sep 3, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Sep 3, 2025

Uh oh!

gemini-code-assist bot Sep 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (const auto port = process->udp_proxy_port? process->udp_proxy_port:get_proxy_port_udp(process); port.has_value())
	if (const auto port = (process->udp_proxy_port ? process->udp_proxy_port : get_proxy_port_udp(process)); port.has_value())

	if (const auto port = process->tcp_proxy_port? process->tcp_proxy_port:get_proxy_port_tcp(process); port.has_value())
	if (const auto port = (process->tcp_proxy_port ? process->tcp_proxy_port : get_proxy_port_tcp(process)); port.has_value())

		std::optional<uint16_t> tcp_proxy_port = std::nullopt; // Optional tcp proxy port if the process is associated with a proxy
		std::optional<uint16_t> udp_proxy_port = std::nullopt; // Optional udp proxy port if the process is associated with a proxy

	// Cache miss - search through patterns
	// Search for a matching process pattern

Uh oh!

feat: integrate caching into network_process objects #87

feat: integrate caching into network_process objects #87

Uh oh!

Conversation

wiresock commented Sep 3, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants