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

Implement per-session parsed statement cache #728

Merged
merged 7 commits into from
Dec 17, 2022

Conversation

vbergeron
Copy link

An attempt to solve #496.

The parse statement cache is modeled after the Describe one in term of API / implementation and visibility.
I added handles to get all the data in cache in order to handle the session cleanup.

The cleanup is done at the Protocol[F] level.

I finally decided to keep the Parse.apply in Resource to not break a lot of code, this could be handled as a follow up pr.

Open to all suggestions :)

@vbergeron
Copy link
Author

Lacking some test for now, but opening it to review.

@vbergeron vbergeron marked this pull request as ready for review November 28, 2022 20:55
@vbergeron
Copy link
Author

So I ended up preserving the prepare method on the toplevel session API, and used prepareAndCache for non-resource usage of prepared statement.

Still wondering about how many tests to add. Would a ParseCacheTest (based on the DescribeCacheTest) be sufficient ?

@codecov
Copy link

codecov bot commented Dec 17, 2022

Codecov Report

Merging #728 (fcbc8fd) into main (782dd29) will decrease coverage by 2.36%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##             main     #728      +/-   ##
==========================================
- Coverage   87.18%   84.82%   -2.37%     
==========================================
  Files         111      124      +13     
  Lines        1506     1667     +161     
  Branches      105      124      +19     
==========================================
+ Hits         1313     1414     +101     
- Misses        193      253      +60     
Impacted Files Coverage Δ
...re/shared/src/main/scala/util/StatementCache.scala 62.50% <50.00%> (-1.79%) ⬇️
modules/core/shared/src/main/scala/Session.scala 55.55% <60.00%> (+0.33%) ⬆️
...ore/shared/src/main/scala/net/protocol/Parse.scala 91.66% <80.00%> (-8.34%) ⬇️
...re/shared/src/main/scala/data/SemispaceCache.scala 100.00% <100.00%> (ø)
...ules/core/shared/src/main/scala/net/Protocol.scala 82.35% <100.00%> (+1.10%) ⬆️
...e/shared/src/main/scala/net/protocol/Prepare.scala 100.00% <100.00%> (ø)
...core/shared/src/main/scala/net/message/Close.scala 80.00% <0.00%> (-20.00%) ⬇️
...ore/shared/src/main/scala/net/protocol/Close.scala 83.33% <0.00%> (-16.67%) ⬇️
modules/core/shared/src/main/scala/Fragment.scala 100.00% <0.00%> (ø)
...core/shared/src/main/scala/net/MessageSocket.scala 100.00% <0.00%> (ø)
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mpilquist mpilquist merged commit fd3b400 into typelevel:main Dec 17, 2022
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.

3 participants