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

Expunge stale entries in InternalLoggerRegistry #3430

Open
vy opened this issue Jan 31, 2025 · 4 comments
Open

Expunge stale entries in InternalLoggerRegistry #3430

vy opened this issue Jan 31, 2025 · 4 comments
Assignees
Labels
enhancement Additions or updates to features performance Issues or PRs that affect performance, throughput, latency, etc.

Comments

@vy
Copy link
Member

vy commented Jan 31, 2025

InternalLoggerRegistry is essentially a Map<MessageFactory, Map<String, WeakReference<Logger>>>. As reported by @tristantarrant in #3399, stale entries are not expunged in its current form. That is, the inner map does not get compacted for entries whose associated WeakReferences are reclaimed by the garbage collector. This might cause high memory usage if an excessive amount of ephemeral loggers get registered.

Reclaimed references can be tracked by a ReferenceQueue passed to the WeakReference::new and processed as follows:

private final ReferenceQueue<Logger> staleLoggerRefs = new ReferenceQueue<>();

private void expungeStaleEntries() {
    Reference<? extends Logger> loggerRef;
    while ((loggerRef = staleLoggerRefs.poll()) != null) {
        Logger logger = loggerRef.get();
        if (logger != null) {
            removeLogger(logger);
        }
    }
}

When to run expungeStaleEntries() should be decided with care, since removeLogger() mutates the registry map.

On method invocation

All ILR methods (getLogger(), computeIfAbsent(), etc.) can first try to expunge stale entries.
This approach is exercised by WeakHashMap itself and in #3427.

Pros:

  • Implies no changes besides ILR

Cons:

  • Read-only operations, which are accessed most, might try to acquire the write-lock.
  • If ILR is not accessed, stale entries will still be kept. Consider an application where all Loggers are assigned, and a temporary operation creates excessive amount of loggers and completes. All created temporary Logger instances will be reclaimed by GC, yet will continue taking space for the associated Map.Entry<String, WeakReference<Logger>>.

By a janitor thread

ILR can start a janitor thread listening on the ReferenceQueue<Logger>.

Cons:

  • ILR must have a life cycle (to start/stop the janitor thread) managed by LoggerContext.
@vy vy added enhancement Additions or updates to features performance Issues or PRs that affect performance, throughput, latency, etc. labels Jan 31, 2025
@Suvrat1629
Copy link
Contributor

Hi, I'd like to work on this issue. My approach was using a ReferenceQueue to track stale WeakReference entries and expunge them before operations like getLogger() and computeIfAbsent(). Does this approach sit right?

@vy
Copy link
Member Author

vy commented Feb 17, 2025

I'd like to work on this issue. My approach was using a ReferenceQueue to track stale WeakReference entries and expunge them before operations like getLogger() and computeIfAbsent(). Does this approach sit right?

@Suvrat1629, certainly – this is the "On method invocation" approach I described in the ticket. Please try to keep your changes at minimum. Even though it will be difficult, please try to add tests.

@Suvrat1629
Copy link
Contributor

@vy I will try my best with the test cases but where do I put the test case file? And is there any example I can refer to?

@vy
Copy link
Member Author

vy commented Feb 18, 2025

I will try my best with the test cases but where do I put the test case file? And is there any example I can refer to?

@Suvrat1629, try searching for InternalLoggerRegistry tests first, please. Once you've found it, you can create an extra test (e.g., InternalLoggerRegistryGcTest) next to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additions or updates to features performance Issues or PRs that affect performance, throughput, latency, etc.
Projects
None yet
Development

No branches or pull requests

2 participants