Skip to content
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.

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

Reduce per-row allocations #130

Merged
merged 1 commit into from
Mar 24, 2024
Merged

Conversation

zolstein
Copy link
Contributor

@zolstein zolstein commented Mar 22, 2024

Modify several internal functions to avoid allocating unnecessary memory. This commit employs three strategies:

  • Allocate a scans slice only when scanning the first row, and store it inside the RowScanner to reuse on subsequent rows.
  • When scanning into slices, use more sophistocated reflection code to extend the slie while avoid temporary slice header allocations.
  • When scanning into slices of non-pointers, scan directly into the slice index, rather than allocate a new value and copy after scanning.

I used a simple but (hopefully) vaguely representative benchmarking program to measure the impact.

Hyperfine shows an end-to-end runtime reduction of 40%.


# Without change
$ go build main.go && hyperfine --warmup=1 "./main -bench"           12:25:17
Benchmark 1: ./main -bench
  Time (mean ± σ):     15.984 s ±  0.115 s    [User: 93.516 s, System: 20.190 s]
  Range (min … max):   15.825 s … 16.258 s    10 runs
 
# With change
$ go build main.go && hyperfine --warmup=1 "./main -bench"              12:28:54
Benchmark 1: ./main -bench
  Time (mean ± σ):      9.186 s ±  0.094 s    [User: 60.400 s, System: 8.723 s]
  Range (min … max):    9.020 s …  9.385 s    10 runs

This also reduced allocated objects (when making 1000 queries returning 1000 rows each) by roughly 3 million and allocated space by ~150MB. (I can share the pprof SVGs if you want.)

Here's the (condensed) text: of the benchmark, if you want to replicate.

type Person struct {
	Name string
	Age  sql.Null[int64]
}

type Record struct {
	ID int64
	Person
	CreatedAt time.Time
}

func main() {
	ctx := context.Background()

	numThreads := 10
	numIters := 5000
	var wg sync.WaitGroup
	wg.Add(numThreads)
	for t := 0; t < numThreads; t++ {
		go func(t int) {
			_, err := runQueries(ctx, numIters)
			if err != nil {
				fmt.Fprintf(os.Stderr, "QueryRow failed: %v\n", err)
				os.Exit(1)
			}
			wg.Done()
		}(t)
	}
	wg.Wait()
}

func runQueries(ctx context.Context, numIters int) ([]Record, error) {
	conn, err := pgx.Connect(ctx, "postgres://zolstein:password@localhost:5432/db")
	if err != nil {
		fmt.Fprintf(os.Stderr, "Unable to connect to database: %v\n", err)
		os.Exit(1)
	}
	defer conn.Close(ctx)

	records := make([]Record, 1000)
	for i := 0; i < numIters; i++ {
		records = records[:0]
		rows, err := conn.Query(ctx, "select id, name, age, created_at from people") // Returns 1000 rows
		_ = rows
		if err != nil {
			return nil, err
		}
		err = pgxscan.ScanAll(&records, rows)
		if err != nil {
			return nil, err
		}
		_ = records
	}
	return records, nil
}

Modify several internal functions to avoid allocating unnecessary
memory. This commit employs three strategies:

- Allocate a scans slice only when scanning the first row, and store it
  inside the RowScanner to reuse on subsequent rows.
- When scanning into slices, use more sophistocated reflection code to
  extend the slie while avoid temporary slice header allocations.
- When scanning into slices of non-pointers, scan directly into the
  slice index, rather than allocate a new value and copy after scanning.
Copy link

codecov bot commented Mar 24, 2024

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 80.72%. Comparing base (981a63a) to head (9e763fa).

Files Patch % Lines
dbscan/dbscan.go 87.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   80.41%   80.72%   +0.30%     
==========================================
  Files           5        5              
  Lines         531      555      +24     
==========================================
+ Hits          427      448      +21     
- Misses         89       92       +3     
  Partials       15       15              
Flag Coverage Δ
unittests 80.72% <92.50%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@georgysavva georgysavva merged commit 041a992 into georgysavva:master Mar 24, 2024
5 checks passed
@georgysavva
Copy link
Owner

Thank you for this PR. It looks very good!

@georgysavva
Copy link
Owner

@zolstein here is the new release with the change: https://github.com/georgysavva/scany/releases/tag/v2.1.1

@zolstein zolstein deleted the reduce-allocs branch March 24, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants