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 Size Calculation #2052

Merged
merged 2 commits into from
Jul 17, 2024
Merged

Fix Size Calculation #2052

merged 2 commits into from
Jul 17, 2024

Conversation

CascadingRadium
Copy link
Member

@CascadingRadium CascadingRadium commented Jul 16, 2024

  • Fix Size() calculation to account for dynamic entries of the struct, to ensure that accurate information is passed on to the memory throttler

Copy link
Member

@abhinavdangeti abhinavdangeti left a comment

Choose a reason for hiding this comment

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

@CascadingRadium would you throw in a detailed description and a much better commit header on what's exactly being addressed here and why.

@CascadingRadium
Copy link
Member Author

CascadingRadium commented Jul 16, 2024

I noticed a few structs had incorrect Size values due to some of the its fields not being accounted for...i fixed them to account such fields, like for example

  1. len(string) should be there in the sizeOfBytes calculation because reflect.SizeOf will only incorporate the size of empty string -> vectorOptimizedFor and vectorSimilarity.
  2. For numeric and datetime fields the PrefixCoded value, (a []byte), was not accounted for in the Size Calculation because of which the lastDocSize in Batch was slightly off since it did not include the length of the PrefixCoded value

abhinavdangeti
abhinavdangeti previously approved these changes Jul 16, 2024
@abhinavdangeti abhinavdangeti added this to the v2.4.2 milestone Jul 16, 2024
@CascadingRadium CascadingRadium merged commit 58a7264 into master Jul 17, 2024
9 checks passed
@CascadingRadium CascadingRadium deleted the size branch July 17, 2024 05:57
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.

2 participants