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

fix: MixedBatchWriter should nil the slice instead of zeroing #1553

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

disq
Copy link
Member

@disq disq commented Mar 5, 2024

Helps with memory.

Benched with #1552, old:

BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10    	     826	   1445595 ns/op	      1393 bytes/op
BenchmarkWriterMemory/MixedBatchWriter_defaults
BenchmarkWriterMemory/MixedBatchWriter_defaults-10              	     670	   1504799 ns/op	      1393 bytes/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10         	       8	 128966609 ns/op	    172785 bytes/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                   	       9	 118630315 ns/op	     60355 bytes/op

new:

BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10    	     690	   1476479 ns/op	      1401 bytes/op
BenchmarkWriterMemory/MixedBatchWriter_defaults
BenchmarkWriterMemory/MixedBatchWriter_defaults-10              	     687	   1539707 ns/op	      1401 bytes/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10         	       8	 129154755 ns/op	    173592 bytes/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                   	       9	 111416773 ns/op	     44588 bytes/op

Copy link

github-actions bot commented Mar 5, 2024

⏱️ Benchmark results

  • Glob-8 ns/op: 92.42

@github-actions github-actions bot added fix and removed fix labels Mar 5, 2024
@disq disq added the automerge label Mar 5, 2024
@kodiakhq kodiakhq bot merged commit f565da8 into cloudquery:main Mar 5, 2024
7 checks passed
@disq disq deleted the fix/mixedbatchwriter-nil-slice branch March 5, 2024 17:56
kodiakhq bot pushed a commit that referenced this pull request Mar 6, 2024
🤖 I have created a release *beep* *boop*
---


## [4.32.1](v4.32.0...v4.32.1) (2024-03-06)


### Bug Fixes

* **deps:** Update golang.org/x/exp digest to 814bf88 ([#1540](#1540)) ([e80fb24](e80fb24))
* **deps:** Update google.golang.org/genproto/googleapis/api digest to df926f6 ([#1541](#1541)) ([9d8a3ec](9d8a3ec))
* **deps:** Update google.golang.org/genproto/googleapis/rpc digest to df926f6 ([#1543](#1543)) ([9315c16](9315c16))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.19.1 ([#1549](#1549)) ([3112739](3112739))
* **deps:** Update module github.com/klauspost/compress to v1.17.7 ([#1544](#1544)) ([4e04027](4e04027))
* **deps:** Update module github.com/klauspost/cpuid/v2 to v2.2.7 ([#1545](#1545)) ([0fff7ed](0fff7ed))
* **deps:** Update module github.com/tdewolff/minify/v2 to v2.20.18 ([#1546](#1546)) ([45fa641](45fa641))
* **deps:** Update module github.com/ugorji/go/codec to v1.2.12 ([#1547](#1547)) ([cd3488a](cd3488a))
* **deps:** Update module google.golang.org/grpc to v1.62.0 ([#1550](#1550)) ([9ccec98](9ccec98))
* **deps:** Update module google.golang.org/grpc to v1.62.0 ([#1551](#1551)) ([d907120](d907120))
* MixedBatchWriter should nil the slice instead of zeroing ([#1553](#1553)) ([f565da8](f565da8))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@candiduslynx
Copy link
Contributor

  1. When you use nil you immediately introduce a bug with cap (see: https://github.com/cloudquery/plugin-sdk/blob/main/writers/mixedbatchwriter/mixedbatchwriter.go#L177, https://github.com/cloudquery/plugin-sdk/blob/main/writers/mixedbatchwriter/mixedbatchwriter.go#L209)
  2. I don't see much improvements here either, maybe I'm not looking correctly
nil

BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              2724 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              2463 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              2053 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1732 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1639 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1802 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1660 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1673 ns/op              1393 bytes/op         1393 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1739 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1722 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1760 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1646 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1472 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1618 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1692 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1748 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1870 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1796 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1574 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1828 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            124192 ns/op            174793 bytes/op       232125 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            121860 ns/op            183238 bytes/op       232116 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            126347 ns/op            175911 bytes/op       232126 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            122443 ns/op            180146 bytes/op       232120 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            121118 ns/op            180755 bytes/op       232108 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            125056 ns/op            183281 bytes/op       232103 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            123030 ns/op            186821 bytes/op       232120 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            121279 ns/op            178714 bytes/op       232097 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            124958 ns/op            178596 bytes/op       232105 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            125754 ns/op            183995 bytes/op       232115 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            123916 ns/op            173773 bytes/op       232106 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            125387 ns/op            185542 bytes/op       232122 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            122312 ns/op            181158 bytes/op       232114 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            122467 ns/op            181971 bytes/op       232112 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            125174 ns/op            183753 bytes/op       232113 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            122004 ns/op            180405 bytes/op       232121 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            123294 ns/op            176132 bytes/op       232100 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            124500 ns/op            180189 bytes/op       232115 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            127433 ns/op            174755 bytes/op       232112 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            123012 ns/op            176072 bytes/op       232121 B/op       2049 allocs/op

[:0]

BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              2776 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              2582 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              2208 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1536 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1614 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1646 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1754 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1702 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1724 ns/op              1393 bytes/op         1393 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_batch10k_bytes100M-10                1000              1762 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1834 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1693 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1804 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1797 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1739 ns/op              1721 bytes/op         1720 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1832 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1727 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1700 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1726 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_defaults-10                          1000              1771 ns/op              1721 bytes/op         1721 B/op         23 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            123076 ns/op            179582 bytes/op       232118 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            124413 ns/op            177396 bytes/op       232121 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            122790 ns/op            179931 bytes/op       232106 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            121242 ns/op            182132 bytes/op       232107 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            120752 ns/op            182754 bytes/op       232109 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            122812 ns/op            181840 bytes/op       232118 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            123734 ns/op            176544 bytes/op       232118 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            124438 ns/op            181274 bytes/op       232122 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            123256 ns/op            182356 bytes/op       231774 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_batch10k_bytes100M-10           1000            122667 ns/op            178061 bytes/op       232104 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            122845 ns/op            182142 bytes/op       232100 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            123886 ns/op            182779 bytes/op       232111 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            123729 ns/op            182779 bytes/op       232096 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            123632 ns/op            175236 bytes/op       231790 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            122319 ns/op            181572 bytes/op       232103 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            132980 ns/op            179910 bytes/op       232122 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            124032 ns/op            177858 bytes/op       232121 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            124370 ns/op            181475 bytes/op       232089 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            124490 ns/op            179209 bytes/op       232124 B/op       2049 allocs/op
BenchmarkWriterMemory/MixedBatchWriter_wide_defaults-10                     1000            119278 ns/op            182648 bytes/op       232124 B/op       2049 allocs/op

@candiduslynx
Copy link
Contributor

Yeah, seems like pg v7.3.7 (without this change) can write 8K rows/sec in the AWS src scenario, while 7.3.8 (with this fix) can handle write only around 3K rows/sec

@disq
Copy link
Member Author

disq commented Apr 22, 2024

Looks like this introduced a bug regarding cap since we don't have the set batch size capacity any more and the memory "improvements" might be related to that. In this case the batches would be flushed when the element size reaches 2^n (possibly 1, but if we don't check/call flush all the time it can go up in 2^n increments, 1, 2, 4, 8, ...)

kodiakhq bot pushed a commit that referenced this pull request Apr 22, 2024
Fixes the issue with `cap` used on `nil` slice introduced in #1553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants