Skip to content
This repository was archived by the owner on Jan 25, 2024. It is now read-only.

Avoid copies when passing binary parameters #1

Merged
merged 5 commits into from
Sep 5, 2022

Conversation

robx
Copy link
Collaborator

@robx robx commented Sep 5, 2022

This is haskellari#22, filed against this forked repo. As with hasql-pool, hoping to get this upstream eventually, but let's move forward like this for now.

Currently, the library copies every input parameter when passing it to libpq via Data.ByteString.useAsCString. This PR avoids those copies for the case of binary parameters, by using Data.ByteString.Unsafe.unsafeUseAsCString.

I claim that this is safe because:

  • libpq doesn't require binary parameters to be zero-terminated, they are passed with length
  • libpq treats parameter input as read-only
  • ByteString uses pinned memory

There are a few preparatory commits to factor out duplicate code to withParams, and to avoid a bit of unnecessary work there.

Context is PostgREST/postgrest#2261. We see significant decrease of memory use with this change: PostgREST/postgrest#2349

robx added 4 commits July 4, 2022 10:03
There were two identical copies each of the parameter mangling
code.
Instead of building a list of 'Int' in the fold, and then mapping
'toEnum' over it, build a list of 'CInt' directly.
We encode various arrays of the same length through 'withArray'.
Each of these computes the length, so we might as well take the
length from one of these.
@robx robx requested a review from steve-chavez September 5, 2022 13:30
@robx robx merged commit 3d64a40 into PostgREST:master Sep 5, 2022
robx added a commit to robx/postgrest that referenced this pull request Sep 5, 2022
robx added a commit to robx/postgrest that referenced this pull request Sep 5, 2022
robx added a commit to robx/postgrest that referenced this pull request Sep 5, 2022
robx added a commit to PostgREST/postgrest that referenced this pull request Sep 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants