X Tutup
The Wayback Machine - https://web.archive.org/web/20260105003829/https://github.com/github/codeql/pull/15957
Skip to content

Conversation

@tamasvajk
Copy link
Contributor

@tamasvajk tamasvajk commented Mar 18, 2024

This PR reduces the number of extraction messages and compiler diagnostics that get stored in the database. I've added some env variables that can fine tune the limits, but I haven't introduced an extractor option for them.

@github-actions github-actions bot added the C# label Mar 18, 2024
protected override void Populate(TextWriter trapFile)
{
// The below doesn't limit the extractor messages to the exact limit, but it's good enough.
Interlocked.Increment(ref messageCount);

Check notice

Code scanning / CodeQL

Static field written by instance method

Write to static field from instance method, property, or constructor.
{
internal class CompilerDiagnostic : FreshEntity
{
private static readonly int limit = EnvironmentVariables.TryGetExtractorOption<int>("COMPILER_DIAGNOSTIC_LIMIT") ?? 1000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we introduce an extractor option for this? What should be the name for the option?

@tamasvajk tamasvajk marked this pull request as ready for review March 20, 2024 13:08
@tamasvajk tamasvajk requested a review from a team as a code owner March 20, 2024 13:08
hvitved
hvitved previously approved these changes Mar 20, 2024
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM; one suggestion.

@tamasvajk tamasvajk added the no-change-note-required This PR does not need a change note label Mar 22, 2024
@tamasvajk
Copy link
Contributor Author

@hvitved Can you have a look at the new commits in this PR.
The DCA experiment is failing with OOM, but I think that's on variant 0. Also the results show that there are many messages that are not logged any longer. It's not just buildless that's impacted.

@tamasvajk tamasvajk merged commit d6374f6 into github:main Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup