Skip to content

feature flag lazy runeDocSection decoding#312

Merged
keegancsmith merged 2 commits intomasterfrom
k/disable-lazy
Apr 6, 2022
Merged

feature flag lazy runeDocSection decoding#312
keegancsmith merged 2 commits intomasterfrom
k/disable-lazy

Conversation

@keegancsmith
Copy link
Member

@keegancsmith keegancsmith commented Mar 23, 2022

Currently this takes up 30% of production CPU. We are unsure if its worth the tradeoff so we will experiment with enabling vs disabling and measure the impact.

We default to the current behaviour. To disable you set the environment variable ZOEKT_DISABLE_LAZY_DOC_SECTIONS to a non empty value.

Test Plan: locally run zoekt-webserver with and without the feature flag. With the feature flag we should have higher memory usage. Ensure in both cases symbol search works.

Currently this takes up 30% of production CPU. We are unsure if its
worth the tradeoff so we will experiment with enabling vs disabling and
measure the impact.

We default to the current behaviour. To disable you set the environment
variable ZOEKT_DISABLE_LAZY_DOC_SECTIONS to a non empty value.

Test Plan: locally run zoekt-webserver with and without the feature
flag. With the feature flag we should have higher memory usage. Ensure
in both cases symbol search works.
@keegancsmith keegancsmith requested a review from a team April 4, 2022 13:41
@keegancsmith keegancsmith marked this pull request as ready for review April 4, 2022 13:41
Shards: 1,
Documents: 4,
IndexBytes: 308,
IndexBytes: 300,
Copy link
Member

Choose a reason for hiding this comment

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

We could instead udpate memoryUse like this

- sz += 8 * len(d.runeDocSections)
+ sz += 8 * len(d.runeDocSectionsRaw)

Copy link
Member Author

Choose a reason for hiding this comment

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

if its the raw section then it is mmap and isn't part of our stable memory usage.

@keegancsmith keegancsmith merged commit acffe79 into master Apr 6, 2022
@keegancsmith keegancsmith deleted the k/disable-lazy branch April 6, 2022 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants