Improve the dump output of ConcurrentPool #11036
Conversation
Signed-off-by: Ludovic Orban <[email protected]>
| return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), _weak.get(), _strong); | ||
| ConcurrentEntry<P> weakEntry = _weak.get(); | ||
| ConcurrentEntry<P> strongEntry = _strong; | ||
| return "%s@%x{w=%s,s=%s,e=%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry == null ? "null" : "non-null", strongEntry == null ? "null" : "non-null", weakEntry); |
There was a problem hiding this comment.
Why this "null"/"non-null"?
Just print the weak and strong entries.
There was a problem hiding this comment.
No, because when an entry is at rest, both the weak and strong refs are non-null, so we'd print the same info twice. This is what we currently do and it makes the dump slightly confusing.
This is why I've changed the string to specify which ref is null or not (the info we need to troubleshoot weak/strong refs) plus the entry itself, to improve clarity.
There was a problem hiding this comment.
| return "%s@%x{w=%s,s=%s,e=%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry == null ? "null" : "non-null", strongEntry == null ? "null" : "non-null", weakEntry); | |
| return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry, strongEntry == null ? "free" : "held", weakEntry); |
There was a problem hiding this comment.
I don't mind this change. @sbordet would you be happy with it?
There was a problem hiding this comment.
@lorban I don't like "free"/"held" (never did), so I'd prefer "acquired"/"idle" respectively (I want to know if the entry has been acquired, and "free" does not tell me that).
Also, the suggestion uses 4 parameters, but 5 arguments.
There was a problem hiding this comment.
@sbordet But the methods are called hold and free. We never agreed on better names so changing them to something else in the dump is just confusing.
The names have to match the methods. Ok to rename methods.. but idle really doesn't convey the same meaning as hold.
| return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), _weak.get(), _strong); | ||
| ConcurrentEntry<P> weakEntry = _weak.get(); | ||
| ConcurrentEntry<P> strongEntry = _strong; | ||
| return "%s@%x{w=%s,s=%s,e=%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry == null ? "null" : "non-null", strongEntry == null ? "null" : "non-null", weakEntry); |
There was a problem hiding this comment.
| return "%s@%x{w=%s,s=%s,e=%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry == null ? "null" : "non-null", strongEntry == null ? "null" : "non-null", weakEntry); | |
| return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry, strongEntry == null ? "free" : "held", weakEntry); |
| return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), _weak.get(), _strong); | ||
| ConcurrentEntry<P> weakEntry = _weak.get(); | ||
| ConcurrentEntry<P> strongEntry = _strong; | ||
| return "%s@%x{w=%s,s=%s,e=%s}".formatted(this.getClass().getSimpleName(), hashCode(), weakEntry == null ? "null" : "non-null", strongEntry == null ? "null" : "non-null", weakEntry); |
There was a problem hiding this comment.
@lorban I don't like "free"/"held" (never did), so I'd prefer "acquired"/"idle" respectively (I want to know if the entry has been acquired, and "free" does not tell me that).
Also, the suggestion uses 4 parameters, but 5 arguments.
Signed-off-by: Ludovic Orban <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
Clean the
ConcurrentPool.dump()output by including omitted data and removing duplicated data.