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

Raw store #1406

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Raw store #1406

wants to merge 1 commit into from

Conversation

SOF3
Copy link
Contributor

@SOF3 SOF3 commented Feb 11, 2024

Motivation

Described in #1389. To summarize, RawJson has a smaller memory footprint compared to DynamicObject, only requiring 48 bytes of overhead (+ one JSON buffer allocation on heap) for each object for the smallest case.

Solution

Introduce a new raw_json module:

  • The type RawJson is a new ToObjectRef implementation that can be used in reflectors similar to how DynamicObject is used, with efficient access to the object ref fields required for reflector (i.e. namespace, name, resource version and uid)
  • The utility type StrOffset acts like a Range<usize>, providing utilities to convert from/to a substring &str in a JSON buffer
  • The Extra trait provides the ability to cache additional data (offsets or other lightweight data) in a RawJson
  • Extra is implemented by () to indicate that no raw data is requested.

Copy link

codecov bot commented Feb 12, 2024

Codecov Report

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

Project coverage is 74.7%. Comparing base (22cb4a4) to head (f0f322d).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1406     +/-   ##
=======================================
- Coverage   74.9%   74.7%   -0.2%     
=======================================
  Files         78      79      +1     
  Lines       6801    6884     +83     
=======================================
+ Hits        5091    5136     +45     
- Misses      1710    1748     +38     
Files Coverage Δ
kube-runtime/src/raw_json.rs 54.3% <54.3%> (ø)

Motivation explained in kube-rs#1389

Depends on kube-rs#1393 to ease reflector bounds

Signed-off-by: SOFe <sofe2038@gmail.com>
@SOF3 SOF3 marked this pull request as ready for review May 8, 2024 14:16
@SOF3 SOF3 marked this pull request as draft May 8, 2024 14:38
@SOF3
Copy link
Contributor Author

SOF3 commented May 8, 2024

Further work is necessary to refactor kube::Api and/or runtime::watcher such that we can actually create a watcher that uses RawJson only.

@clux
Copy link
Member

clux commented May 9, 2024

AFAIKT there are at least 3 trait bounds we need to think about to make this viable (if we want to integrate it with watcher and Api as is):

  1. watcher' s trait bound on Resource
  2. Api bound on Resource
  3. For Api method ::list and ::watch, the DeserializeOwned bound

The first should be reduced to a trait bound on Lookup pretty easily (i think) without being particularly breaking. Api's Resource bound should only affect constructors, so if can make a new constructor for json usage we should be able to side-step this.

The last one, I am not sure about though.

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.

None yet

2 participants