Skip to content

DataRow without allocation; DataRow as Collection; RowDescription top level #198

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

Merged
merged 3 commits into from
Nov 22, 2021

Conversation

fabianfett
Copy link
Collaborator

This is a cherry pick of #188.

Modifications

  • DataRow and RowDescription have been moved out of the PSQLBackendMessage namespace. This allows us to mark them as @inlinable or @usableFromInline at a later point, without marking everything in PSQLBackendMessage as @inlinable
  • DataRow does not use an internal array for its columns anymore. Instead all read operations are directly done on its ByteBuffer slice.
  • DataRow implements the Collection protocol now.

Result

One allocation fewer per queried row.

@fabianfett fabianfett requested a review from gwynne November 19, 2021 14:05
@fabianfett fabianfett added enhancement New feature or request semver-patch labels Nov 20, 2021
@fabianfett fabianfett force-pushed the ff-dont-alloc-for-row branch from bfa6fdf to 9b87c14 Compare November 22, 2021 07:37
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2021

Codecov Report

Merging #198 (02aa7fc) into main (9967360) will increase coverage by 4.10%.
The diff coverage is 86.04%.

❗ Current head 02aa7fc differs from pull request most recent head e8bc674. Consider uploading reports for the commit e8bc674 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #198      +/-   ##
==========================================
+ Coverage   35.75%   39.85%   +4.10%     
==========================================
  Files         115      115              
  Lines        7741     7795      +54     
==========================================
+ Hits         2768     3107     +339     
+ Misses       4973     4688     -285     
Flag Coverage Δ
unittests 39.85% <86.04%> (+4.10%) ⬆️

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

Impacted Files Coverage Δ
...esNIO/Connection/PostgresConnection+Database.swift 0.00% <0.00%> (ø)
Sources/PostgresNIO/New/PSQLChannelHandler.swift 58.45% <ø> (+10.49%) ⬆️
Sources/PostgresNIO/New/PSQLConnection.swift 31.55% <0.00%> (+1.94%) ⬆️
Sources/PostgresNIO/New/PSQLRow.swift 0.00% <0.00%> (ø)
Sources/PostgresNIO/New/PSQLRowStream.swift 0.00% <0.00%> (ø)
Sources/PostgresNIO/New/PSQLTask.swift 35.13% <ø> (ø)
Sources/PostgresNIO/New/Messages/DataRow.swift 95.94% <95.52%> (-4.06%) ⬇️
...nection State Machine/ConnectionStateMachine.swift 54.82% <100.00%> (+8.88%) ⬆️
...tion State Machine/ExtendedQueryStateMachine.swift 65.26% <100.00%> (+7.78%) ⬆️
...n State Machine/PrepareStatementStateMachine.swift 53.39% <100.00%> (+4.85%) ⬆️
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9967360...e8bc674. Read the comment docs.

Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

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

A few minor nits and questions, nothing serious.

@fabianfett fabianfett force-pushed the ff-dont-alloc-for-row branch from c0a6ab4 to 797055c Compare November 22, 2021 08:12
@fabianfett fabianfett requested a review from gwynne November 22, 2021 08:23
@fabianfett fabianfett force-pushed the ff-dont-alloc-for-row branch from 797055c to 02aa7fc Compare November 22, 2021 08:28
Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

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

LGTM now 🙂

@fabianfett fabianfett merged commit 1042870 into vapor:main Nov 22, 2021
@VaporBot
Copy link

These changes are now available in 1.6.4

@fabianfett fabianfett deleted the ff-dont-alloc-for-row branch March 13, 2022 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants