Skip to content

use mmap syscall instead of open to reduce baseline memory footprint on startup #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
af-emp-0271 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

use mmap syscall instead of open to reduce baseline memory footprint on startup #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions proxydb/proxydb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package proxydb

import (
"errors"
"os"
"syscall"
Comment on lines +5 to +6
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's any build constraint or platform-specific handling in the repo
rg -n "//go:build|// \+build" --type go
# Check go.mod for target platforms or existing mmap libraries
cat go.mod 2>/dev/null || echo "go.mod not found"

Repository: CloudStuffTech/go-utils

Length of output: 3814

🏁 Script executed:

#!/bin/bash
# Find and read proxydb.go
find . -name "proxydb.go" -type f

Repository: CloudStuffTech/go-utils

Length of output: 87

🏁 Script executed:

#!/bin/bash
# First check the beginning of the file to see build constraints and imports
head -60 proxydb/proxydb.go

Repository: CloudStuffTech/go-utils

Length of output: 1091

🏁 Script executed:

#!/bin/bash
# Search for actual usage of Mmap in the file
rg -n "Mmap|Munmap" proxydb/proxydb.go

Repository: CloudStuffTech/go-utils

Length of output: 193

🏁 Script executed:

#!/bin/bash
# Check first 10 lines specifically for build constraints
head -10 proxydb/proxydb.go

Repository: CloudStuffTech/go-utils

Length of output: 151

🏁 Script executed:

#!/bin/bash
# Check if there's a proxydb_unix.go or similar platform-specific variant
find proxydb -name "*.go" | head -20

Repository: CloudStuffTech/go-utils

Length of output: 85

Platform portability: syscall.Mmap and syscall.Munmap are Unix-specific and will fail on Windows.

The code directly uses syscall.Mmap and syscall.Munmap (lines 41 and 55) without any build constraints or platform-specific handling. These functions are unavailable on Windows and will cause compilation failures. If cross-platform support is required, use golang.org/x/exp/mmap or github.com/edsrzf/mmap-go to provide a portable abstraction.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@proxydb/proxydb.go` around lines 5 - 6, The code currently calls syscall.Mmap
and syscall.Munmap (in proxydb.go) which are Unix-only and break Windows builds;
replace these direct syscall usages with a cross-platform mmap abstraction
(e.g., golang.org/x/exp/mmap or github.com/edsrzf/mmap-go) or split
platform-specific implementations using build tags. Concretely, remove calls to
syscall.Mmap/syscall.Munmap in the function(s) that perform file memory mapping,
introduce an interface or helper (e.g., MapFile/UnmapFile) backed by the chosen
portable package (or by separate _unix.go and _windows.go files with build
constraints), and update callers to use that abstraction so the package compiles
and runs on Windows as well as Unix.


"github.com/etf1/ip2proxy"
)

type Client struct {
dbpath string
DB *ip2proxy.DB
dbpath string
DB *ip2proxy.DB
mmapData []byte
}
Comment on lines 11 to 15
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing Close() method to release mmap resources.

The Client now holds mmapData which is memory-mapped via syscall.Mmap, but there's no corresponding Close() method to call syscall.Munmap. This will leak the memory-mapped region for the lifetime of the process.

Proposed fix: Add a Close method 
+func (c *Client) Close() error {
+	if c.mmapData != nil {
+		err := syscall.Munmap(c.mmapData)
+		c.mmapData = nil
+		c.DB = nil
+		return err
+	}
+	return nil
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type Client struct {
dbpath string
DB *ip2proxy.DB
dbpath string
DB *ip2proxy.DB
mmapData []byte
}
func (c *Client) Close() error {
if c.mmapData != nil {
err := syscall.Munmap(c.mmapData)
c.mmapData = nil
c.DB = nil
return err
}
return nil
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@proxydb/proxydb.go` around lines 11 - 15, Add a Close method on the Client
type that unmaps mmapData and clears the reference; specifically implement func
(c *Client) Close() error that checks if c.mmapData != nil, calls
syscall.Munmap(c.mmapData), handles and returns any error, and then sets
c.mmapData = nil (and optionally c.DB = nil) so resources are released; ensure
callers of the client (where Client is created) call Client.Close() when done.


type Data struct {
Expand All @@ -22,16 +25,52 @@ func NewClient(dbpath string) *Client {
return client
}

func (c *Client) Open() error {
var err error
c.DB, err = ip2proxy.Open(c.dbpath)
func openMmap(c *Client) error {
file, err := os.Open(c.dbpath)
if err != nil {
return err
}
defer file.Close() // Safe to close the file descriptor after mapping

stat, err := file.Stat()
if err != nil {
return err
}

// Map the file directly into virtual memory (Bypassing Go's Heap)
c.mmapData, err = syscall.Mmap(
int(file.Fd()),
0,
int(stat.Size()),
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential integer overflow on 32-bit systems.

int(stat.Size()) truncates the file size on 32-bit systems where int is 32 bits, but stat.Size() returns int64. Files larger than 2GB would cause incorrect behavior.

Proposed fix: Add size validation 
+	size := stat.Size()
+	if size > int64(^uint(0)>>1) { // max int
+		return errors.New("file too large to mmap on this platform")
+	}
+
 	// Map the file directly into virtual memory (Bypassing Go's Heap)
 	c.mmapData, err = syscall.Mmap(
 		int(file.Fd()),
 		0,
-		int(stat.Size()),
+		int(size),
 		syscall.PROT_READ,  // Read-only memory
 		syscall.MAP_SHARED, // Share with OS page cache
 	)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
int(stat.Size()),
size := stat.Size()
if size > int64(^uint(0)>>1) { // max int
return errors.New("file too large to mmap on this platform")
}
// Map the file directly into virtual memory (Bypassing Go's Heap)
c.mmapData, err = syscall.Mmap(
int(file.Fd()),
0,
int(size),
syscall.PROT_READ, // Read-only memory
syscall.MAP_SHARED, // Share with OS page cache
)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@proxydb/proxydb.go` at line 44, int(stat.Size()) can overflow on 32-bit
systems because stat.Size() is int64; update the code to avoid truncation by
either (a) changing the receiving variable/types to int64 and use stat.Size()
directly, or (b) if an int is required, validate the size before casting: import
math, check if stat.Size() > int64(math.MaxInt) and return an error when too
large, otherwise safely cast with int(stat.Size()); refer to the call using
stat.Size() and the current int(...) cast to locate the change.

syscall.PROT_READ, // Read-only memory
syscall.MAP_SHARED, // Share with OS page cache
)
if err != nil {
return err
}

c.DB, err = ip2proxy.FromBytes(c.mmapData)
if err != nil {
c.DB = nil
// Clean up the OS memory if parsing fails
syscall.Munmap(c.mmapData)
c.mmapData = nil
return err
}
return nil
}

func (c *Client) Open() error {
err := openMmap(c)
return err
// var err error
// c.DB, err = ip2proxy.Open(c.dbpath)
// if err != nil {
// c.DB = nil
// return err
// }
// return nil
}

func (c *Client) proxyName(proxyType ip2proxy.ProxyType) string {
if proxyType == ip2proxy.ProxyVPN {
return "vpn"
Expand Down