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

BAAS-11692: add mem usage for setObject and func objects #121

Merged
merged 5 commits into from
Jun 14, 2024

Conversation

Gabri3l
Copy link

@Gabri3l Gabri3l commented Jun 12, 2024

No description provided.

builtin_set.go Outdated Show resolved Hide resolved
builtin_set.go Show resolved Hide resolved
builtin_set.go Show resolved Hide resolved
builtin_set.go Outdated

// We can use samplesVisited instead of an index since we iterate using
// iterNext
for item := so.m.iterFirst; item != nil && samplesVisited < uint64(totalItems); item = item.iterNext {

Choose a reason for hiding this comment

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

[q] looks like totalItems is the size of the map but we only increment sampleVisited when its divisible by the sample size which means this case should never be true samplesVisited < uint64(totalItems) (unless you are accounting for size of 1?). Can we remove that check samplesVisited < uint64(totalItems):

	for item := so.m.iterFirst; item != nil; item = item.iterNext {

Copy link
Author

Choose a reason for hiding this comment

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

you're right, even with a step of 1 we would have a nil item and get out I believe so this is not needed.

@Gabri3l Gabri3l requested a review from jwongmongodb June 14, 2024 19:13
Copy link

@jwongmongodb jwongmongodb left a comment

Choose a reason for hiding this comment

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

LGTM

@Gabri3l Gabri3l merged commit b528e7b into mongodb-forks:realm Jun 14, 2024
2 of 6 checks passed
@Gabri3l Gabri3l deleted the BAAS-11692 branch June 14, 2024 19:44
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