Add std.experimental.logger.asynclogger #3194
Conversation
| { | ||
| super(lv); | ||
| _logger = cast(shared Logger)(logger); | ||
| _tid = spawn(&(spawnedFunc), _logger); |
There was a problem hiding this comment.
Why the parens? It looks as if you want to take the address of the return value.
|
Rest LGTM. |
| SysTime timestamp; | ||
| /// the message of the log message | ||
| string msg; | ||
| } |
There was a problem hiding this comment.
this struct feels wrong. could you mix that with LogEntry into something like
struct LogEntryBase {
// all but Tid
}
struct LogEntry {
LogEntryBase base;
Tid threadId;
}
|
|
||
| /// thread id of the log message | ||
| Tid threadId; | ||
| /// A reference to the $(D Logger) used to create this $(D LogEntry) | ||
| Logger logger; |
There was a problem hiding this comment.
holding a ref to the Logger seams wrong. as LogEntry is only a aggregation of Log Data.
There was a problem hiding this comment.
This field was in original LogEntry and LogEntry was/is protected. Potentially this change can break user code. May be it is better to remove this field by separate commit, just in case.
|
@Groterik ping |
|
Sorry for the delay, it seems that |
|
Now |
| import std.concurrency; | ||
| public import std.typecons : Unique, unique; | ||
|
|
||
| /** The $(D AsyncLogger) processes logs asynchronously by using another $(D Logger) implementation |
There was a problem hiding this comment.
the first sentences is confusing:
the act of using another logger does not make it asynchronously
please rewrite
|
ping |
|
merge failed - please rebase. |
|
ping @Groterik - it still needs a rebase and addressing of the comments.. |
|
This PR has been inactive for almost a year, I suggest this be closed due to inactivity. |
Async logging seems to be important. Anyone interested in picking this up or at least putting to dub? |
|
I'd be interested in seeing this available - at least on dub. In its current form it requires a change to Phobos to work however: std.experimental.logger.core (splitting LogEntry into LogEntryBase without Tid). |
|
You could takeover this PR by recreating this patch in another PR. @Groterik seams to be inactive with this PR. |
|
I'm looking into this to see whether I can see a way to do it with no Phobos change first. |
|
@burner I have some questions on the design of std.experimental.logger, mainly relating to threading aspects. What is the best forum for these, here? dlang forums? PM? somewhere else? |
I think such a discussion could be interesting for more people, so how about general in the NG? |
Agreed @Groterik, @kookman or anyone else: please reopen to revive this PR. Also I created a Bugzilla enhancement request, s.t. it's not forgotten. @burner I think this would be a pretty cool enhancement to the logging module (and is probably one of the missing bits to move it out of |
Asynchronous wrapper for std.experimental.logger. It sends log-message to special logging thread where the message is logged finally.
Discussion: http://forum.dlang.org/thread/lcsjtxorbbagmbvbllns@forum.dlang.org