www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.experimental.logger formal review round 3

reply "Dicebot" <public dicebot.lv> writes:
Previous review thread : 
http://forum.dlang.org/post/zhvmkbahrqtgkptdlcvh forum.dlang.org

Previous voting thread (also contains discussion in the end) : 
http://forum.dlang.org/post/vbotavcclttrgvzcjjia forum.dlang.org

Code : https://github.com/D-Programming-Language/phobos/pull/1500

Important changes since last review:
- new approach for compile-time log level filtering
- thread-safe API (by Marco Leise, commits 3b32618..e71f317)
- documentation enhancements all over the place
- more  nogc annotations
- "raw" log overload that expects pre-formatted message and all 
metadata (file/line/function) as run-time function arguments

(anything I have missed Robert?)

Usual process : 2 weeks for checking out if there are any 
critical issues that are likely to prevent successful voting, 
write a comment if you need more time for review, focus on API 
issues.
Sep 28 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 28 September 2014 at 12:24:23 UTC, Dicebot wrote:
 Previous review thread : 
 http://forum.dlang.org/post/zhvmkbahrqtgkptdlcvh forum.dlang.org

 Previous voting thread (also contains discussion in the end) : 
 http://forum.dlang.org/post/vbotavcclttrgvzcjjia forum.dlang.org

 Code : 
 https://github.com/D-Programming-Language/phobos/pull/1500

 Important changes since last review:
 - new approach for compile-time log level filtering
 - thread-safe API (by Marco Leise, commits 3b32618..e71f317)
 - documentation enhancements all over the place
 - more  nogc annotations
 - "raw" log overload that expects pre-formatted message and all 
 metadata (file/line/function) as run-time function arguments

 (anything I have missed Robert?)
I added more unittests, but unfortunately didn't find any bugs. The "raw" log overload is actually a template function that has one template parameter, the value you want to log. That's all, I think
Sep 28 2014
next sibling parent reply Marco Leise <Marco.Leise gmx.de> writes:
This module is going to be the go-to logging in D.
Every other D logging library will extend on it, and libraries
will pass their messages through std.logger's `stdlog`.

It is very important that all of you who plan on using logging
in D or know about specific logger implementations take their
time and see if what they have in mind can be implemented on
the API foundations that std.logger provides. Our community
lacks a real logging guru and needs the collective experience.

If you don't have time to experiment, but believe that some
use case must be covered just describe it in a few words.
(Especially effects on API design, performance or threading.)
Sep 29 2014
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Monday, 29 September 2014 at 10:30:21 UTC, Marco Leise wrote:
 If you don't have time to experiment, but believe that some
 use case must be covered just describe it in a few words.
Two things: 1. Timestamps with millisecond precision or better? 2. Creating one log file per day? I.e. built-in logrotate. If it's not built-in, would it be difficult to add on top?
Sep 29 2014
next sibling parent "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 29 September 2014 at 10:41:06 UTC, Vladimir Panteleev 
wrote:
 On Monday, 29 September 2014 at 10:30:21 UTC, Marco Leise wrote:
 If you don't have time to experiment, but believe that some
 use case must be covered just describe it in a few words.
Two things: 1. Timestamps with millisecond precision or better?
You have all that is in Datetime.
 2. Creating one log file per day? I.e. built-in logrotate. If 
 it's not built-in, would it be difficult to add on top?
No, as said in the last review thread there are to many solution on different platforms. But you can build the one you need easily.
Sep 29 2014
prev sibling next sibling parent Marco Leise <Marco.Leise gmx.de> writes:
Am Mon, 29 Sep 2014 10:41:05 +0000
schrieb "Vladimir Panteleev" <vladimir thecybershadow.net>:

 Two things:
 
 1. Timestamps with millisecond precision or better?
 
 2. Creating one log file per day? I.e. built-in logrotate. If 
 it's not built-in, would it be difficult to add on top?
Basically your logger gets passed all the basic information (file, line, function, module, log level, Tid, timestamp as `SysTime` (up to hnsecs precision), message and the original logger). You decide how to handle it, whether you pass it on to another logger, open a file or keep it in memory. -- Marco
Sep 29 2014
prev sibling parent "Dicebot" <public dicebot.lv> writes:
On Monday, 29 September 2014 at 10:41:06 UTC, Vladimir Panteleev 
wrote:
 2. Creating one log file per day? I.e. built-in logrotate. If 
 it's not built-in, would it be difficult to add on top?
I see logger customization explained in top-level docs of https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d Do you feel something needs to be added there?
Sep 29 2014
prev sibling next sibling parent Marco Leise <Marco.Leise gmx.de> writes:
The thread-safety changes in short: Every Logger is now taking
a lock when a public method is invoked and then calls the user
overridable methods with the lock taken.

* When you implement a new logger, thread-safety is already
  taken care of.
* Taking a lock in a single-threaded context is practically
  free compared to the actual logging. (So don't worry.)
* It is not possible to enter a Logger with multiple threads
  and provide more fine-grained thread synchronization. In
  particular only one thread at a time can call a global log
  function like `debug(...)` or `error(...)`.

Note: This is different from the original intention to have
Loggers implement thread safety if they need it and a stopgap
measure looking for use cases that need modifications.

--
Marco
Sep 29 2014
prev sibling parent reply "Kevin Lamonte" <kevindotlamnodotonte gmail.com> writes:
I haven't tested it yet, but have two questions anyway:

1. I did not see any reference to the use of Clock.currTime(), 
which on the last round accounted for about 90% of the total time 
spent in a log call.  Reference: 
https://issues.dlang.org/show_bug.cgi?id=13433 .  (This is the 
difference between logging-and-filtering ~100k logs/sec and ~1M 
logs/sec for loggers that use criteria other than logLevel for 
filtering messages.)  Same question for this cycle:  Does 
std.logger API need a method for clients or subclasses to 
change/defer/omit the call to Clock.currTime?  Or defer for a 
change in std.datetime?

2. We have Tid in the API.  What about Fiber and Thread?  If we 
can only pick one, I would vote for Thread rather than Tid, as 
Tid's currently have no way to be uniquely identified in a 
logging message.  Reference: 
https://issues.dlang.org/show_bug.cgi?id=6989

General comment: very nice to see continued progress!
Oct 01 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Wednesday, 1 October 2014 at 10:50:54 UTC, Kevin Lamonte wrote:
 I haven't tested it yet, but have two questions anyway:

 1. I did not see any reference to the use of Clock.currTime(), 
 which on the last round accounted for about 90% of the total 
 time spent in a log call.  Reference: 
 https://issues.dlang.org/show_bug.cgi?id=13433 .  (This is the 
 difference between logging-and-filtering ~100k logs/sec and ~1M 
 logs/sec for loggers that use criteria other than logLevel for 
 filtering messages.)  Same question for this cycle:  Does 
 std.logger API need a method for clients or subclasses to 
 change/defer/omit the call to Clock.currTime?  Or defer for a 
 change in std.datetime?
maybe I should add a disableGetSysTime switch
 2. We have Tid in the API.  What about Fiber and Thread?  If we 
 can only pick one, I would vote for Thread rather than Tid, as 
 Tid's currently have no way to be uniquely identified in a 
 logging message.  Reference: 
 https://issues.dlang.org/show_bug.cgi?id=6989

 General comment: very nice to see continued progress!
I'm gone take a closer look
Oct 01 2014
next sibling parent reply Marco Leise <Marco.Leise gmx.de> writes:
Am Wed, 01 Oct 2014 12:49:29 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:

 maybe I should add a disableGetSysTime switch
CLOCK_REALTIME_COARSE / CLOCK_REALTIME_FAST should be explored. On Linux you can't expect finer resolution than the kernel Hz, for FreeBSD I only found mention of 10ms resolution. If you format log messages with 2 digits time precision anyway, you don't need the precise version. If you disable time completely, what would the LogEntry contain as the time stamp? SysTime.init? -- Marco
Oct 01 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Wednesday, 1 October 2014 at 14:24:52 UTC, Marco Leise wrote:
 Am Wed, 01 Oct 2014 12:49:29 +0000
 schrieb "Robert burner Schadek" <rburners gmail.com>:

 maybe I should add a disableGetSysTime switch
CLOCK_REALTIME_COARSE / CLOCK_REALTIME_FAST should be explored.
good pointer, but what about Win and Mac
 If you disable time completely, what would the LogEntry
 contain as the time stamp? SysTime.init?
That was my first idea.
Oct 01 2014
next sibling parent reply Marco Leise <Marco.Leise gmx.de> writes:
Am Wed, 01 Oct 2014 15:05:53 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:

 On Wednesday, 1 October 2014 at 14:24:52 UTC, Marco Leise wrote:
 Am Wed, 01 Oct 2014 12:49:29 +0000
 schrieb "Robert burner Schadek" <rburners gmail.com>:

 maybe I should add a disableGetSysTime switch
CLOCK_REALTIME_COARSE / CLOCK_REALTIME_FAST should be explored.
good pointer, but what about Win and Mac
Windows 2000 had some function that returns 4ms accurate time, I hope it is implemented like CLOCK_REALTIME_COARSE. OS X ... oh well. Don't know. Just declare the "fast timer" a hint, I guess. Like when you ask for anti-aliasing in OpenGL and the implementation is free to decide if it can or want's to deliver. So it turns into: "I need a sub-second timestamp, but make it as fast as possible on the target OS". Maybe some day Apple will copy CLOCK_REALTIME_FAST from FreeBSD.
 If you disable time completely, what would the LogEntry
 contain as the time stamp? SysTime.init?
That was my first idea.
-- Marco
Oct 01 2014
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 10/1/14 11:53 AM, Marco Leise wrote:
 Am Wed, 01 Oct 2014 15:05:53 +0000
 schrieb "Robert burner Schadek" <rburners gmail.com>:

 On Wednesday, 1 October 2014 at 14:24:52 UTC, Marco Leise wrote:
 Am Wed, 01 Oct 2014 12:49:29 +0000
 schrieb "Robert burner Schadek" <rburners gmail.com>:

 maybe I should add a disableGetSysTime switch
CLOCK_REALTIME_COARSE / CLOCK_REALTIME_FAST should be explored.
good pointer, but what about Win and Mac
Windows 2000 had some function that returns 4ms accurate time, I hope it is implemented like CLOCK_REALTIME_COARSE.
I think it wouldn't be prudent to explore Windows options until we prove the windows currTime is slow like the Linux version. On Mac, gettimeofday is used. I don't know that it's necessarily slow, but I don't know of a better way to get the wall clock time. -Steve
Oct 01 2014
parent "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Wednesday, 1 October 2014 at 16:10:41 UTC, Steven 
Schveighoffer wrote:
 I think it wouldn't be prudent to explore Windows options until 
 we prove the windows currTime is slow like the Linux version.

 On Mac, gettimeofday is used. I don't know that it's 
 necessarily slow, but I don't know of a better way to get the 
 wall clock time.
x86 has a builtin time stamp counter, executes faster than the fastest system call. http://en.wikipedia.org/wiki/Time_Stamp_Counter http://en.wikipedia.org/wiki/High_Precision_Event_Timer
Oct 02 2014
prev sibling parent =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig rejectedsoftware.com> writes:
Am 01.10.2014 17:05, schrieb Robert burner Schadek:
 On Wednesday, 1 October 2014 at 14:24:52 UTC, Marco Leise wrote:
 Am Wed, 01 Oct 2014 12:49:29 +0000
 schrieb "Robert burner Schadek" <rburners gmail.com>:

 If you disable time completely, what would the LogEntry
 contain as the time stamp? SysTime.init?
That was my first idea.
It should be noted that SysTime.init.toString() currently crashes (at least some other methods, too).
Oct 02 2014
prev sibling parent =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig rejectedsoftware.com> writes:
Am 01.10.2014 14:49, schrieb Robert burner Schadek:
 On Wednesday, 1 October 2014 at 10:50:54 UTC, Kevin Lamonte wrote:
 I haven't tested it yet, but have two questions anyway:

 1. I did not see any reference to the use of Clock.currTime(), which
 on the last round accounted for about 90% of the total time spent in a
 log call.  Reference: https://issues.dlang.org/show_bug.cgi?id=13433
 .  (This is the difference between logging-and-filtering ~100k
 logs/sec and ~1M logs/sec for loggers that use criteria other than
 logLevel for filtering messages.)  Same question for this cycle:  Does
 std.logger API need a method for clients or subclasses to
 change/defer/omit the call to Clock.currTime?  Or defer for a change
 in std.datetime?
maybe I should add a disableGetSysTime switch
Don't know if this is still the case, but calling Clock.currTime() used to allocate on each call, whereas Clock.currTime(UTC()) didn't and thus was orders of magnitudes faster.
Oct 02 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 1 October 2014 at 10:50:54 UTC, Kevin Lamonte wrote:
 2. We have Tid in the API.  What about Fiber and Thread?  If we 
 can only pick one, I would vote for Thread rather than Tid, as 
 Tid's currently have no way to be uniquely identified in a 
 logging message.  Reference: 
 https://issues.dlang.org/show_bug.cgi?id=6989
In my opinion only solution that scales is to provide same ID as one used by std.concurrency - it can be thread, fiber, process or pretty much anything. There are many possible threading abstractions and all can't be easily supported, makes sense to stick to one considered standard.
Oct 01 2014
parent reply "Kevin Lamonte" <kevindotlamnodotonte gmail.com> writes:
On Wednesday, 1 October 2014 at 13:37:43 UTC, Dicebot wrote:
 On Wednesday, 1 October 2014 at 10:50:54 UTC, Kevin Lamonte 
 wrote:
 2. We have Tid in the API.  What about Fiber and Thread?  If 
 we can only pick one, I would vote for Thread rather than Tid, 
 as Tid's currently have no way to be uniquely identified in a 
 logging message.  Reference: 
 https://issues.dlang.org/show_bug.cgi?id=6989
In my opinion only solution that scales is to provide same ID as one used by std.concurrency - it can be thread, fiber, process or pretty much anything. There are many possible threading abstractions and all can't be easily supported, makes sense to stick to one considered standard.
Long-term this sounds very reasonable. But the problem at the moment is that std.concurrency provides nothing that can be used to distinguish threads in a log file. Tid cannot be converted to a unique string or vice versa, nor can one Tid.join() to ensure that a child Thread has completed before closing the log file(s) and exiting main(). For boring end-user applications Threads are today's most common denominator. Would PR https://github.com/D-Programming-Language/phobos/pull/1910 provide a way given a Tid to determine: a) What underlying concurrency model it is using (Thread, Fiber, process, future)? b) Uniquely identify that structure (Thread ID string, Fiber address string, process ID, something else)? c) Be capable of using that identifying immutable (because it needs to be send()able to another Tid writing to network/file/etc) string-representable thing to find the original Tid again? A+B is necessary for using std.logger to debug concurrent applications, C is a very nice-to-have that comes up periodically.
Oct 02 2014
parent reply "Sean Kelly" <sean invisibleduck.org> writes:
On Thursday, 2 October 2014 at 10:37:02 UTC, Kevin Lamonte wrote:
 Would PR 
 https://github.com/D-Programming-Language/phobos/pull/1910 
 provide a way given a Tid to determine: a) What underlying 
 concurrency model it is using (Thread, Fiber, process, future)? 
 b) Uniquely identify that structure (Thread ID string, Fiber 
 address string, process ID, something else)?  c) Be capable of 
 using that identifying immutable (because it needs to be 
 send()able to another Tid writing to network/file/etc) 
 string-representable thing to find the original Tid again?  A+B 
 is necessary for using std.logger to debug concurrent 
 applications, C is a very nice-to-have that comes up 
 periodically.
register() is meant to provide a means of referring to a thread. But the relevant thing there is finding a thread by role, not by instance. So if a thread doing a known job terminates, a new one can spawn and register under the same name so proper operation can continue. Having an identifier for logging is a bit different. Would using the MessageBox address be sufficient? I'd be happy to add a Tid.id property that returns a value like this. I'd rather not try to generate a globally unique identifier though (that would probably mean a UUID, which is long and expensive to generate).
Oct 05 2014
parent reply "Kevin Lamonte" <kevindotlamnodotonte gmail.com> writes:
On Sunday, 5 October 2014 at 17:06:06 UTC, Sean Kelly wrote:

 Having an identifier for logging is a bit different.  Would 
 using the MessageBox address be sufficient?  I'd be happy to 
 add a Tid.id property that returns a value like this.  I'd 
 rather not try to generate a globally unique identifier though 
 (that would probably mean a UUID, which is long and expensive 
 to generate).
I think Tid.id returning the MessageBox address would be fine for logging purposes. The main value is being able to distinguish messages coming in at the same time from multiple threads. Even if a MessageBox address was re-used by a new receive()er after a previous one exited I doubt it would confuse users very much. I thing that a doc on Tid.id like "this value will not be the same as any other Tid currently existing, but might be the same as a Tid that has exited previously" would be sufficient.
Oct 05 2014
parent "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Sunday, 5 October 2014 at 23:40:32 UTC, Kevin Lamonte wrote:
 On Sunday, 5 October 2014 at 17:06:06 UTC, Sean Kelly wrote:

 Having an identifier for logging is a bit different.  Would 
 using the MessageBox address be sufficient?  I'd be happy to 
 add a Tid.id property that returns a value like this.  I'd 
 rather not try to generate a globally unique identifier though 
 (that would probably mean a UUID, which is long and expensive 
 to generate).
I think Tid.id returning the MessageBox address would be fine for logging purposes. The main value is being able to distinguish messages coming in at the same time from multiple threads. Even if a MessageBox address was re-used by a new receive()er after a previous one exited I doubt it would confuse users very much. I thing that a doc on Tid.id like "this value will not be the same as any other Tid currently existing, but might be the same as a Tid that has exited previously" would be sufficient.
A moving GC can affect the address, too, but for your purpose it would still be fine, you just mustn't put too much significance into the actual value.
Oct 07 2014
prev sibling next sibling parent reply "deadalnix" <deadalnix gmail.com> writes:
On Sunday, 28 September 2014 at 12:24:23 UTC, Dicebot wrote:
 Previous review thread : 
 http://forum.dlang.org/post/zhvmkbahrqtgkptdlcvh forum.dlang.org

 Previous voting thread (also contains discussion in the end) : 
 http://forum.dlang.org/post/vbotavcclttrgvzcjjia forum.dlang.org

 Code : 
 https://github.com/D-Programming-Language/phobos/pull/1500

 Important changes since last review:
 - new approach for compile-time log level filtering
 - thread-safe API (by Marco Leise, commits 3b32618..e71f317)
 - documentation enhancements all over the place
 - more  nogc annotations
 - "raw" log overload that expects pre-formatted message and all 
 metadata (file/line/function) as run-time function arguments

 (anything I have missed Robert?)

 Usual process : 2 weeks for checking out if there are any 
 critical issues that are likely to prevent successful voting, 
 write a comment if you need more time for review, focus on API 
 issues.
It has been mentioned in the comment already, but log4j like approach seems better. Also, it is sad that output range are not leveraged to format/sample/filter/select output of the logger.
Sep 29 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 30 September 2014 at 00:19:00 UTC, deadalnix wrote:
 It has been mentioned in the comment already, but log4j like
 approach seems better. Also, it is sad that output range are not
 leveraged to format/sample/filter/select output of the logger.
yeah log4j is totally awesome and shows IMO how pretty something can be build on top of std.logger. Even though it uses the non-range LogEntry from std.logger.
Sep 30 2014
prev sibling next sibling parent "ponce" <contact gam3sfrommars.fr> writes:
On Sunday, 28 September 2014 at 12:24:23 UTC, Dicebot wrote:
 Previous review thread : 
 http://forum.dlang.org/post/zhvmkbahrqtgkptdlcvh forum.dlang.org

 Previous voting thread (also contains discussion in the end) : 
 http://forum.dlang.org/post/vbotavcclttrgvzcjjia forum.dlang.org

 Code : 
 https://github.com/D-Programming-Language/phobos/pull/1500

 Important changes since last review:
 - new approach for compile-time log level filtering
 - thread-safe API (by Marco Leise, commits 3b32618..e71f317)
 - documentation enhancements all over the place
 - more  nogc annotations
 - "raw" log overload that expects pre-formatted message and all 
 metadata (file/line/function) as run-time function arguments

 (anything I have missed Robert?)

 Usual process : 2 weeks for checking out if there are any 
 critical issues that are likely to prevent successful voting, 
 write a comment if you need more time for review, focus on API 
 issues.
Upgraded my logger to 0.3.0. I like that I don't have to make them thread-safe myself. I vote 'yes' again.
Sep 29 2014
prev sibling next sibling parent reply Marco Leise <Marco.Leise gmx.de> writes:
How would I typically log an exception?
I thought of one line per exception in the chain plus a full
trace for the last exception in the chain.

So the messages would be like:
  Initialization failed.
  Could not load configuration.
  File "~/.config/app/settings.ini" no found.
  Stack trace (outer to inner):
    _Dmain_yadda_yadda+120
    Config.__ctor(=E2=80=A6)+312
    =E2=80=A6
    File.__ctor(=E2=80=A6)+12

So far so good, but I'm stuck at this line of code (where `e`
is a Throwable):

  error(e.msg, e.line, e.file, funcName, prettyFuncName, moduleName);

I don't know how to get at the function and module name where
the exception was thrown. I know this stuff is part of the
symbolic debug information, but I would think it is a common
use case of a logger to log exceptions.

Is it? And if so, what do we want to do about it?

--=20
Marco
Oct 02 2014
next sibling parent "Kevin Lamonte" <kevindotlamnodotonte gmail.com> writes:
On Thursday, 2 October 2014 at 18:11:26 UTC, Marco Leise wrote:
 How would I typically log an exception?
We could add a Throwable reference in LogEntry and some overrides. But how exception stack traces appear in the output (multiple lines, all-on-one line, e.msg only, following the chain, etc.) should not be firmly set here: we can provide a reasonable baseline in FileLogger, but other Logger subclasses will definitely want to do different things. Another issue is that in practice one will often want to log at debug an exception they are about to throw (libraries), but log at error anything they actually catch (applications). Should we include those use cases too? This skirts very close to the "policy, not mechanism" line, but might be worth it. Proposal: LogEntry { ... Throwable throwable; /// Optional Throwable bool caught; /// If true, then throwable was logged via caught/caughtf } void throwing(Throwable t, string msg) {...} void throwingf(Throwable t, string formatMsg, A... args) {...} void caught(Throwable t, string msg) {...} void caughtf(Throwable t, string formatMsg, A... args) {...} Thoughts?
Oct 03 2014
prev sibling parent "Robert burner Schadek" <rburners gmail.com> writes:
On Thursday, 2 October 2014 at 18:11:26 UTC, Marco Leise wrote:
 How would I typically log an exception?
 I thought of one line per exception in the chain plus a full
 trace for the last exception in the chain.

 So the messages would be like:
   Initialization failed.
   Could not load configuration.
   File "~/.config/app/settings.ini" no found.
   Stack trace (outer to inner):
     _Dmain_yadda_yadda+120
     Config.__ctor(…)+312
     …
     File.__ctor(…)+12

 So far so good, but I'm stuck at this line of code (where `e`
 is a Throwable):

   error(e.msg, e.line, e.file, funcName, prettyFuncName, 
 moduleName);

 I don't know how to get at the function and module name where
 the exception was thrown. I know this stuff is part of the
 symbolic debug information, but I would think it is a common
 use case of a logger to log exceptions.
I guess that should be part of the Exception. I have no idea how to get __FUCTION__ of and Exception from inside another function.
 Is it? And if so, what do we want to do about it?
Oct 05 2014
prev sibling next sibling parent reply =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig rejectedsoftware.com> writes:
I still think that there should be the two predefined log levels "debug" 
(for developer related diagnostics) and "diagnostic" (for end user 
related diagnostics) between "trace" and "info". This is important for 
interoperability of different libraries, so that they have predictable 
debug output.

But independent of that, there should really be a function for safely 
generating the user defined intermediate log levels, maybe like this:

     LogLevel raiseLevel(LogLevel base_level, ubyte increment)
     {
         assert(base_level is a valid base level);
         assert(base_level + increment smaller than the next base level);
         return cast(LogLevel)(base_level + increment);
     }

     // ok
     enum notice = raiseLevel(LogLevel.info, 16);

     // error, overlaps with the next base level
     enum suspicious = raiseLevel(LogLevel.info, 32);

Casting to an enum type is a pretty blunt operation, so it should at 
least be made as safe as possible.
Oct 02 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Thursday, 2 October 2014 at 20:15:32 UTC, Sönke Ludwig wrote:
 I still think that there should be the two predefined log 
 levels "debug" (for developer related diagnostics) and 
 "diagnostic" (for end user related diagnostics) between "trace" 
 and "info". This is important for interoperability of different 
 libraries, so that they have predictable debug output.

 But independent of that, there should really be a function for 
 safely generating the user defined intermediate log levels, 
 maybe like this:

     LogLevel raiseLevel(LogLevel base_level, ubyte increment)
     {
         assert(base_level is a valid base level);
         assert(base_level + increment smaller than the next 
 base level);
         return cast(LogLevel)(base_level + increment);
     }

     // ok
     enum notice = raiseLevel(LogLevel.info, 16);

     // error, overlaps with the next base level
     enum suspicious = raiseLevel(LogLevel.info, 32);

 Casting to an enum type is a pretty blunt operation, so it 
 should at least be made as safe as possible.
from log4d /// Aliases for debugX and fineX functions alias debug1 = defaultLogFunction!(Log4DLogger.LOG_LEVEL_DEBUG1); alias debug2 = defaultLogFunction!(Log4DLogger.LOG_LEVEL_DEBUG2); alias debug3 = defaultLogFunction!(Log4DLogger.LOG_LEVEL_DEBUG3); but adding a raiseLevel function should be no problem
Oct 05 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
I don't see critical objections so far and this will move to 
voting stage this weekend. Please hurry up if you want to say 
something bad :)
Oct 10 2014
next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Saturday, 11 October 2014 at 03:41:08 UTC, Dicebot wrote:
 I don't see critical objections so far and this will move to 
 voting stage this weekend. Please hurry up if you want to say 
 something bad :)
Attributes need to be applied thoroughly. Even if most uses will be through the base class `Logger`, it's still useful to have stronger guarantees through a derived class reference. This is particularly important because it's an important design decision to choose which attributes to apply to `Logger`'s methods. trusted is used everywhere instead of properly using safe and minimized trusted. I think this is the third time I point this out... The multiloggers are a complete mess. There's both `ArrayLogger` and `MultiLogger`, and while `ArrayLogger` has simple O(n) operations, `MultiLogger` is a disaster: insertion iterates all elements twice and sorts the entire collection on every call, and removal iterates all elements once, then does binary search twice. Once using `SortedRange`'s search, and once using its own binary search algorithm. It also contains debug code that writes to stdout. Neither type adheres to the Phobos container concept, instead the underlying array is exposed as a public, undocumented field. `string` is used instead of `const(char)[]` for search and removal operations. The implementation of `Logger` has several performance problems. `Logger` provides default behaviour that allocates GC memory multiple times for even the simplest log messages through the `Appender`. I don't think this behaviour should be encouraged by putting it in the root logger class, and besides, it can be made much more intelligent than just using a new appender for each message. Another issue is that the way it's written currently, `writeLogPart` is called a lot more often than necessary, without any opportunity for optimization within `formattedWrite`, thus `FileLogger` is doomed to write to the underlying file character-by-character in easily reproducible circumstances (e.g. log a range of characters); this issue probably doesn't affect the API though. `Logger` has a bunch of public and documented `*Impl` functions... Some other line comments I posted a while ago have not been addressed.
Oct 10 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 11 October 2014 at 04:31:17 UTC, Jakob Ovrum wrote:
 On Saturday, 11 October 2014 at 03:41:08 UTC, Dicebot wrote:
 I don't see critical objections so far and this will move to 
 voting stage this weekend. Please hurry up if you want to say 
 something bad :)
Attributes need to be applied thoroughly. Even if most uses will be through the base class `Logger`, it's still useful to have stronger guarantees through a derived class reference. This is particularly important because it's an important design decision to choose which attributes to apply to `Logger`'s methods. trusted is used everywhere instead of properly using safe and minimized trusted. I think this is the third time I point this out... The multiloggers are a complete mess. There's both `ArrayLogger` and `MultiLogger`, and while `ArrayLogger` has simple O(n) operations, `MultiLogger` is a disaster: insertion iterates all elements twice and sorts the entire collection on every call, and removal iterates all elements once, then does binary search twice. Once using `SortedRange`'s search, and once using its own binary search algorithm. It also contains debug code that writes to stdout. Neither type adheres to the Phobos container concept, instead the underlying array is exposed as a public, undocumented field. `string` is used instead of `const(char)[]` for search and removal operations.
The latest std.container.Array broke the code anyway, so it is due for a rewrite anyway.
 The implementation of `Logger` has several performance 
 problems. `Logger` provides default behaviour that allocates GC 
 memory multiple times for even the simplest log messages 
 through the `Appender`. I don't think this behaviour should be 
 encouraged by putting it in the root logger class, and besides, 
 it can be made much more intelligent than just using a new 
 appender for each message.
Well, to have ultra simple thread-safe sub classing (which is an important part of the design), this was the price. This being said. Doing it nogc yourself if you know the output is very easy as shown in FileLogger.
 Another issue is that the way it's written currently, 
 `writeLogPart` is called a lot more often than necessary, 
 without any opportunity for optimization within 
 `formattedWrite`, thus `FileLogger` is doomed to write to the 
 underlying file character-by-character in easily reproducible 
 circumstances (e.g. log a range of characters); this issue 
 probably doesn't affect the API though.
Again, by design. To allow user created structured logging, this is necessary.
 `Logger` has a bunch of public and documented `*Impl` 
 functions...
see my other post
 Some other line comments I posted a while ago have not been 
 addressed.
I will recheck github
Oct 11 2014
parent reply Marco Leise <Marco.Leise gmx.de> writes:
Am Sat, 11 Oct 2014 12:23:10 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:

 On Saturday, 11 October 2014 at 04:31:17 UTC, Jakob Ovrum wrote:
 The implementation of `Logger` has several performance=20
 problems. `Logger` provides default behaviour that allocates GC=20
 memory multiple times for even the simplest log messages=20
 through the `Appender`. I don't think this behaviour should be=20
 encouraged by putting it in the root logger class, and besides,=20
 it can be made much more intelligent than just using a new=20
 appender for each message.
=20 Well, to have ultra simple thread-safe sub classing (which is an=20 important part of the design), this was the price. This being=20 said. Doing it nogc yourself if you know the output is very easy=20 as shown in FileLogger.
I had the same feeling as Jakob about an `Appender` already in the base class and would have expected a bare bones abstract class + a batteries included version using `Appender`. (A bit like Java's =E2=80=A6Listener and =E2=80=A6Adapter classes.) That seems more clean to me in a representational fashion. Technically we can just ignore the extra field... It also seems legit to reduce pressure on the GC, by resetting the `Appender` instead of nulling it. --=20 Marco
Oct 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 11 October 2014 at 23:37:42 UTC, Marco Leise wrote:
 I had the same feeling as Jakob about an `Appender` already
 in the base class and would have expected a bare bones
 abstract class + a batteries included version using `Appender`.
 (A bit like Java's …Listener and …Adapter classes.)
 That seems more clean to me in a representational fashion.
 Technically we can just ignore the extra field...
 It also seems legit to reduce pressure on the GC, by resetting
 the `Appender` instead of nulling it.
What if a Logger down the chain keeps the string around and you overwrite it?
Oct 12 2014
parent Marco Leise <Marco.Leise gmx.de> writes:
Am Sun, 12 Oct 2014 12:07:55 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:

 On Saturday, 11 October 2014 at 23:37:42 UTC, Marco Leise wrote:
 I had the same feeling as Jakob about an `Appender` already
 in the base class and would have expected a bare bones
 abstract class + a batteries included version using `Appender`.
 (A bit like Java's =E2=80=A6Listener and =E2=80=A6Adapter classes.)
 That seems more clean to me in a representational fashion.
 Technically we can just ignore the extra field...
 It also seems legit to reduce pressure on the GC, by resetting
 the `Appender` instead of nulling it.
=20 What if a Logger down the chain keeps the string around and you=20 overwrite it?
That would be bad. --=20 Marco
Oct 14 2014
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2014-10-11 05:41, Dicebot wrote:
 I don't see critical objections so far and this will move to voting
 stage this weekend. Please hurry up if you want to say something bad :)
I think it's unacceptable that the documentation of "defaultLogFunction" and "trace", "info" and so on is merged. Same thing with "memLogFunctions". Do these even need to be exposed? -- /Jacob Carlborg
Oct 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 11 October 2014 at 10:48:00 UTC, Jacob Carlborg 
wrote:
 On 2014-10-11 05:41, Dicebot wrote:
 I don't see critical objections so far and this will move to 
 voting
 stage this weekend. Please hurry up if you want to say 
 something bad :)
I think it's unacceptable that the documentation of "defaultLogFunction" and "trace", "info" and so on is merged. Same thing with "memLogFunctions". Do these even need to be exposed?
this has been used make user defined LogLevel log functions, like trace1(...), trace2(...)
Oct 11 2014
parent Jacob Carlborg <doob me.com> writes:
On 2014-10-11 14:08, Robert burner Schadek wrote:

 this has been used make user defined LogLevel log functions, like

 trace1(...), trace2(...)
I still don't like the merge of the documentation. -- /Jacob Carlborg
Oct 11 2014
prev sibling next sibling parent "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 11 October 2014 at 03:41:08 UTC, Dicebot wrote:
 I don't see critical objections so far and this will move to 
 voting stage this weekend. Please hurry up if you want to say 
 something bad :)
MultiLogger got a new simpler impl.
Oct 11 2014
prev sibling next sibling parent "Ola Fosheim Grostad" <ola.fosheim.grostad+dlang gmail.com> writes:
On Saturday, 11 October 2014 at 03:41:08 UTC, Dicebot wrote:
 I don't see critical objections so far and this will move to 
 voting stage this weekend. Please hurry up if you want to say 
 something bad :)
I don't think I will use it for the reasons I stated in the previous thread, so no point in saying anything bad.
Oct 11 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
As there was quite some last moment feedback I am giving some 
more time for me to research issues a bit and Robert to address 
them :)
Oct 14 2014
next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Wednesday, 15 October 2014 at 02:54:27 UTC, Dicebot wrote:
 As there was quite some last moment feedback I am giving some 
 more time for me to research issues a bit and Robert to address 
 them :)
The Pareto Principle could be worth mentioning here. We were 80% of the way to a quality interface a long time ago, but the last 20% is taking a disproportionate amount of time to iron out. I think all this criticism is indicative that we're holding this module to a high standard rather than the code being bad, which I think is a very good thing. Thankfully Marco stepped up and provided a solution to the threading problem, so I don't think it's that far off. Apropos threading though, I'm not sure how to consolidate the fact that we're using shared memory without using `shared`. It seems like a failure to have such an intricately designed memory model, then as soon as we do threading in Phobos, we ignore it. I still intend to go through all the documentation and fix things I can spot as soon as the interface is finalized.
Oct 14 2014
parent "Paolo Invernizzi" <paolo.invernizzi no.address> writes:
On Wednesday, 15 October 2014 at 03:45:12 UTC, Jakob Ovrum wrote:
 On Wednesday, 15 October 2014 at 02:54:27 UTC, Dicebot wrote:

 Apropos threading though, I'm not sure how to consolidate the 
 fact that we're using shared memory without using `shared`. It 
 seems like a failure to have such an intricately designed 
 memory model, then as soon as we do threading in Phobos, we 
 ignore it.
I've written several times that having the TDPL Concurrency chapter as a free link in the main page it's some sort of horrible masochism, granted the current state of the implementation of what it's described there... But well ;-/ --- /Paolo
Oct 14 2014
prev sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Wednesday, 15 October 2014 at 02:54:27 UTC, Dicebot wrote:
 As there was quite some last moment feedback I am giving some 
 more time for me to research issues a bit and Robert to address 
 them :)
No need, I fixed the MultiLogger last weekend.
Oct 15 2014
parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Wednesday, 15 October 2014 at 09:25:07 UTC, Robert burner 
Schadek wrote:
 On Wednesday, 15 October 2014 at 02:54:27 UTC, Dicebot wrote:
 As there was quite some last moment feedback I am giving some 
 more time for me to research issues a bit and Robert to 
 address them :)
No need, I fixed the MultiLogger last weekend.
"Fixed" by simply removing the attempt at logarithmic time operations, and still not conforming to the Phobos container interface...
Oct 15 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Wednesday, 15 October 2014 at 10:12:56 UTC, Jakob Ovrum wrote:
 On Wednesday, 15 October 2014 at 09:25:07 UTC, Robert burner 
 Schadek wrote:
 On Wednesday, 15 October 2014 at 02:54:27 UTC, Dicebot wrote:
 As there was quite some last moment feedback I am giving some 
 more time for me to research issues a bit and Robert to 
 address them :)
No need, I fixed the MultiLogger last weekend.
"Fixed" by simply removing the attempt at logarithmic time operations, and still not conforming to the Phobos container interface...
Yes, because other people already complained that nobody will every need a logarithmic time MulitLogger and I always thought that MultiLogger was to complex anyway. I could make the Logger[] array alias this and it would be comforming to Phobos container, but that doesn't really solve the issue does it.
Oct 15 2014
prev sibling next sibling parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 09/28/2014 02:24 PM, Dicebot wrote:
 Important changes since last review:
 - new approach for compile-time log level filtering
What's new here? It still relies on version identifiers to do so. As I said in some earlier review, I think it's a bad idea for a library to rely on version identifiers that are defined in client code. I will only work for templated code and makes it much harder for build tools. In a way it's the equivalent of #define LOG_LEVEL 2 #include <logger.h> I even proposed an alternative that uses type tags instead. http://dpaste.dzfl.pl/95fb6a4e086d
 Usual process : 2 weeks for checking out if there are any critical
 issues that are likely to prevent successful voting, write a comment if
 you need more time for review, focus on API issues.
- Documentation is out of sync. - ArrayLogger seems to do about the same as MultiLogger - Why do loggers have to be classes? - Define a concept (isLogger template) with the requirements. - Add a Logger interface and a loggerObject to wrap structures when polymorphic loggers are needed.
Oct 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 11 October 2014 at 13:16:18 UTC, Martin Nowak wrote:
 On 09/28/2014 02:24 PM, Dicebot wrote:
 Important changes since last review:
 - new approach for compile-time log level filtering
What's new here? It still relies on version identifiers to do so. As I said in some earlier review, I think it's a bad idea for a library to rely on version identifiers that are defined in client code. I will only work for templated code and makes it much harder for build tools. In a way it's the equivalent of #define LOG_LEVEL 2 #include <logger.h>
All that code is contained in 30 line template, That is by far the best working option anybody could come up with
 I even proposed an alternative that uses type tags instead.
 http://dpaste.dzfl.pl/95fb6a4e086d
And I showed that it did not work.
 - Documentation is out of sync.
gh-page is yes, give me 15min
 - ArrayLogger seems to do about the same as MultiLogger
have you read my reply to Jacob
 - Why do loggers have to be classes?
As answered multiply times before, to build log hierarchies.
Oct 11 2014
next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2014-10-11 15:34, Robert burner Schadek wrote:

 - Why do loggers have to be classes?
As answered multiply times before, to build log hierarchies.
What's stopping an interface or class to implement a logging concept? -- /Jacob Carlborg
Oct 12 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 12 October 2014 at 09:07:37 UTC, Jacob Carlborg wrote:
 On 2014-10-11 15:34, Robert burner Schadek wrote:

 - Why do loggers have to be classes?
As answered multiply times before, to build log hierarchies.
What's stopping an interface or class to implement a logging concept?
Same as last time: Logger[], Logger without a LogLevel not real useful IMO, (new) no thread safety by default
Oct 12 2014
parent reply "Martin Nowak" <code dawg.eu> writes:
On Sunday, 12 October 2014 at 12:06:44 UTC, Robert burner Schadek 
wrote:
 What's stopping an interface or class to implement a logging 
 concept?
Same as last time: Logger[], Logger without a LogLevel not real useful IMO, (new) no thread safety by default
I don't understand your answer. Do you have a link to your last response.
Oct 24 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 24 October 2014 at 11:01:40 UTC, Martin Nowak wrote:
 On Sunday, 12 October 2014 at 12:06:44 UTC, Robert burner 
 Schadek wrote:
 What's stopping an interface or class to implement a logging 
 concept?
Same as last time: Logger[], Logger without a LogLevel not real useful IMO, (new) no thread safety by default
I don't understand your answer. Do you have a link to your last response.
You can not tell if the Logger will log a message, because you can't know its LogLevel. It is not thread safe because the interface can't have an implementation. Therefore the default implementation is not thread safe.
Oct 24 2014
parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/24/2014 02:16 PM, Robert burner Schadek wrote:
 On Friday, 24 October 2014 at 11:01:40 UTC, Martin Nowak wrote:
 On Sunday, 12 October 2014 at 12:06:44 UTC, Robert burner Schadek wrote:
 What's stopping an interface or class to implement a logging concept?
Same as last time: Logger[], Logger without a LogLevel not real useful IMO, (new) no thread safety by default
I don't understand your answer. Do you have a link to your last response.
You can not tell if the Logger will log a message, because you can't know its LogLevel. It is not thread safe because the interface can't have an implementation. Therefore the default implementation is not thread safe.
Well then the concept is that a Logger has some method that returns the LogLevel (might be a runtime value). Where is the problem? We use this for empty in ranges for example.
Oct 26 2014
prev sibling next sibling parent reply "Martin Nowak" <code dawg.eu> writes:
On Saturday, 11 October 2014 at 13:34:29 UTC, Robert burner 
Schadek wrote:
 All that code is contained in 30 line template, That is by far 
 the best working option anybody could come up with
 I even proposed an alternative that uses type tags instead.
 http://dpaste.dzfl.pl/95fb6a4e086d
And I showed that it did not work.
I never got a reply on this. Let's please try to solve this problem. Depending on the leakage of version identifiers from client to library code (only templates) has severe issues. For example a template isn't reinstantiated, if it was already instatiated in another module (allinst switch). Also version identifiers work globally so you'll probably turn on logging in all libraries you use. http://forum.dlang.org/post/lrf362$tkn$1 digitalmars.com
 - Documentation is out of sync.
gh-page is yes, give me 15min
Thanks
Oct 24 2014
parent "Martin Nowak" <code dawg.eu> writes:
On Friday, 24 October 2014 at 10:59:43 UTC, Martin Nowak wrote:
 And I showed that it did not work.
Found it http://forum.dlang.org/post/hmzfcxlafwlgoovuweak forum.dlang.org
Oct 24 2014
prev sibling next sibling parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/11/2014 03:34 PM, Robert burner Schadek wrote:
 - Why do loggers have to be classes?
As answered multiply times before, to build log hierarchies.
You can easily create classes from structs, that's what I said 2 lines below.
 Add a Logger interface and a loggerObject to wrap structures when 
polymorphic loggers are needed.
Oct 26 2014
parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/26/2014 11:14 PM, Martin Nowak wrote:

 As answered multiply times before, to build log hierarchies.
You can easily create classes from structs, that's what I said 2 lines below.
And you could also log into a tuple of logger structs without polymorphism.
Oct 26 2014
prev sibling parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/11/2014 03:34 PM, Robert burner Schadek wrote:

 As answered multiply times before, to build log hierarchies.
You don't need classes for hierarchies. You need them for runtime polymorphism. We're already building complex hierarchies with Ranges, same can be done for loggers, see http://dpaste.dzfl.pl/2538c3b5d287#line-140.
Oct 26 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
Will start review round in ~2 days.

I am very sorry for delay :(
Oct 24 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 24 October 2014 at 09:53:57 UTC, Dicebot wrote:
 Will start review round in ~2 days.

 I am very sorry for delay :(
No problem the PR has been open since Aug. 2013, one or two weeks more or less don't really matter anymore ;)
Oct 24 2014
parent reply "Dicebot" <public dicebot.lv> writes:
Jut for the reference, my position on current state of things as 
review manager is this:

I agree with some of mentioned concerns and would like to see 
those fixed. However, I don't think any of those are truly 
critical and this proposal has been hanging there for just too 
long. In my opinion it will be more efficient to resolve any 
remainining issues in follow-up pull requests on case by case 
basis then forcing Robert to do it himself - there will be some 
time left before next release to break a thing or two before 
public statement is made.

Because of that I am going to start voting despite some arguments 
being still in process. I hope that won't cause any tension.
Oct 25 2014
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2014-10-25 18:43, Dicebot wrote:
 Jut for the reference, my position on current state of things as review
 manager is this:

 I agree with some of mentioned concerns and would like to see those
 fixed. However, I don't think any of those are truly critical and this
 proposal has been hanging there for just too long. In my opinion it will
 be more efficient to resolve any remainining issues in follow-up pull
 requests on case by case basis then forcing Robert to do it himself -
 there will be some time left before next release to break a thing or two
 before public statement is made.

 Because of that I am going to start voting despite some arguments being
 still in process. I hope that won't cause any tension.
As long as the changes don't require a complete redesign. There was some talk about using templates instead of classes. -- /Jacob Carlborg
Oct 26 2014
prev sibling next sibling parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/25/2014 06:43 PM, Dicebot wrote:
 Because of that I am going to start voting despite some arguments being
 still in process. I hope that won't cause any tension.
The dependency on external version identifiers in phobos is still a complete bummer and there are still many implementation issues. One reason why this is taking so long is that there were many issues some of which still need to be addressed.
Oct 26 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 26 October 2014 at 22:12:20 UTC, Martin Nowak wrote:
 On 10/25/2014 06:43 PM, Dicebot wrote:
 Because of that I am going to start voting despite some 
 arguments being
 still in process. I hope that won't cause any tension.
The dependency on external version identifiers in phobos is still a complete bummer and there are still many implementation issues. One reason why this is taking so long is that there were many issues some of which still need to be addressed.
it is not really a dependency as the one template that uses the version identifier uses them optionally. please name the issues that are not because of the gc.
Oct 26 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 26 October 2014 at 22:27:55 UTC, Robert burner Schadek 
wrote:
 The dependency on external version identifiers in phobos is 
 still a complete bummer
it is not really a dependency as the one template that uses the version identifier uses them optionally.
And I forgot to add, no better solution presented itself in one year.
Oct 26 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/26/2014 11:29 PM, Robert burner Schadek wrote:
 And I forgot to add, no better solution presented itself in one year.
Well I showed one solution, but reduce it to its essence. If you allow to define a Logger with a LogLevel know at compile time and you statically pass the LogLevel of your message to the logging function you can elide that call. For anything else you need a runtime check. http://dpaste.dzfl.pl/2538c3b5d287
Oct 26 2014
next sibling parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 12:45 AM, Martin Nowak wrote:
 but reduce it to its essence.
but let's reduce
Oct 26 2014
prev sibling next sibling parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 12:45 AM, Martin Nowak wrote:
 If you allow to define a Logger with a LogLevel know at compile time and
 you statically pass the LogLevel of your message to the logging function
 you can elide that call. For anything else you need a runtime check.
You are trying to globally define a LogLevel through the version identifier but that collides with D's separate compilation. So you cannot enable logging in a library that was compiled with StdLoggerDisableLogging. And vice versa you cannot statically disable logging in a library compiled without StdLoggerDisableLogging. Now if you use StdLoggerDisableLogging in your program the effect on the library will depend on whether or not you're calling a templated function or if the compiler inlined certain library function into your program.
Oct 26 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 26 October 2014 at 23:58:14 UTC, Martin Nowak wrote:
 On 10/27/2014 12:45 AM, Martin Nowak wrote:
 If you allow to define a Logger with a LogLevel know at 
 compile time and
 you statically pass the LogLevel of your message to the 
 logging function
 you can elide that call. For anything else you need a runtime 
 check.
You are trying to globally define a LogLevel through the version identifier but that collides with D's separate compilation. So you cannot enable logging in a library that was compiled with StdLoggerDisableLogging. And vice versa you cannot statically disable logging in a library compiled without StdLoggerDisableLogging. Now if you use StdLoggerDisableLogging in your program the effect on the library will depend on whether or not you're calling a templated function or if the compiler inlined certain library function into your program.
It is a good think then that the *DisableLogging versions are only used inside a template that is used inside a templates. Though version statements attached to a phobos compilation should only have impact on the unittest of phobos. Secondly, why would phobos be shipped with certain LogLevel disabled.
Oct 27 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 09:22 AM, Robert burner Schadek wrote:
 It is a good think then that the *DisableLogging versions are only used
 inside a template that is used inside a templates. Though version
 statements attached to a phobos compilation should only have impact on
 the unittest of phobos.

 Secondly, why would phobos be shipped with certain LogLevel disabled.
Well the question is, how can you avoid paying the overhead for logging when you don't need it. Say I want to add tracing/logging to [`parseJson`](http://dlang.org/library/std/json/parseJSON.html) or [`findRoot`](http://dlang.org/phobos/std_numeric.html#.findRoot) then I have no way to get rid of the logLevel overhead with the version identifier and abstract class approach. When the logLevel is know at CT this overhead can be completely eliminated. auto json = parseJSON(data, stdoutLogger!(LogLevel.trace)()); auto json = parseJSON(data, stdoutLogger!(LogLevel.warning)()); auto json = parseJSON(data, NullLogger());
Oct 27 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 27 October 2014 at 20:42:10 UTC, Martin Nowak wrote:
 Say I want to add tracing/logging to 
 [`parseJson`](http://dlang.org/library/std/json/parseJSON.html) 
 or 
 [`findRoot`](http://dlang.org/phobos/std_numeric.html#.findRoot)
This is exactly what is wrong :) Using std.logger inside Phobos itself is a big no. Actually even finding yourself in position where you may want to do it indicates some Phobos design issue.
Oct 27 2014
next sibling parent Brad Roberts via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 10/27/14, 1:49 PM, Dicebot via Digitalmars-d wrote:
 On Monday, 27 October 2014 at 20:42:10 UTC, Martin Nowak wrote:
 Say I want to add tracing/logging to
 [`parseJson`](http://dlang.org/library/std/json/parseJSON.html) or
 [`findRoot`](http://dlang.org/phobos/std_numeric.html#.findRoot)
This is exactly what is wrong :) Using std.logger inside Phobos itself is a big no. Actually even finding yourself in position where you may want to do it indicates some Phobos design issue.
Why? I agree that large parts of phobos have little need for logging, but I question the general statement. What's so special about phobos that suggests that no part of it should ever log? How is it different from many other third party libraries that might be similar in nature? Consider the std.net.curl. I find it entirely reasonable to consider that it would have logging added to it. It could be argued that that part of phobos could/should be removed, but ignore that for now and pretend its a full re-implementation of an http client instead if that helps. Consider the scheduler work that Sean is doing. I'll bet he's got printf's in there at some strategic points for debugging right now. Most of those I can easily see translating into trace level logging. The same holds for any higher level tasks that have any sort of complexities and need a way for developers to witness how those subsystems are executing after the fact. My key point, phobos isn't and shouldn't be considered special here. If it shouldn't be doing logging, then why and why doesn't that same logic apply to almost every other library that developers are going to use? I suspect there's some philosophical differences at play. My 2 cents, Brad
Oct 27 2014
prev sibling next sibling parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 09:49 PM, Dicebot wrote:
 This is exactly what is wrong :) Using std.logger inside Phobos itself
 is a big no. Actually even finding yourself in position where you may
 want to do it indicates some Phobos design issue.
Well the same problem also applies to any other library that I obtain as binary release.
Oct 27 2014
prev sibling parent "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 27 October 2014 at 20:49:35 UTC, Dicebot wrote:
 On Monday, 27 October 2014 at 20:42:10 UTC, Martin Nowak wrote:
 Say I want to add tracing/logging to 
 [`parseJson`](http://dlang.org/library/std/json/parseJSON.html) 
 or 
 [`findRoot`](http://dlang.org/phobos/std_numeric.html#.findRoot)
This is exactly what is wrong :) Using std.logger inside Phobos itself is a big no. Actually even finding yourself in position where you may want to do it indicates some Phobos design issue.
Yes and No. His Logger can came from the user aka. outside of phobos. That might just be valid. But the problem with the design is that he needs to accept every possible Logger. And that either means template of abstract class Logger. The problem with a template is: ``` library code that is given in binary form auto fun(Logger l) { return parseJson(getData(), l); } ``` their is no choice but to pass a class. Meaning you have to wrap the struct Logger with a class proxy. And this will properly develop into a common theme. Allowing struct has one design problem IMO: Either we force to callee to accept Logger as template or we force the caller to wrap his Logger struct with Logger proxy classes. This is because an abstract class is the lowest common denominator in this case. Anyway, I'm pretty sure that Martin and I will never see eye to eye in this discussion. IMO disabling a single Logger through its LogLevel at compile (plus all the extra litter and possible needed wrapping) is no better than creating NullLogger. He thinks the opposite. The problem for me now is, if I add a struct UFCS overload Martin will be happy but somebody else will stream WAT "The struct stuff must go" So please everybody reading this, please give a comment about this.
Oct 27 2014
prev sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 26 October 2014 at 23:45:56 UTC, Martin Nowak wrote:
 On 10/26/2014 11:29 PM, Robert burner Schadek wrote:
 And I forgot to add, no better solution presented itself in 
 one year.
Well I showed one solution, but reduce it to its essence. If you allow to define a Logger with a LogLevel know at compile time and you statically pass the LogLevel of your message to the logging function you can elide that call. For anything else you need a runtime check. http://dpaste.dzfl.pl/2538c3b5d287
And again I'm saying fixing the LogLevel at CT is not good enough. And the other part of the solution uses class just like std.logger. And the hierarchy you're building is also at CT, which is just not gone work, if you don't have ultimate control of all sources.
Oct 27 2014
parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 09:08 AM, Robert burner Schadek wrote:
 And again I'm saying fixing the LogLevel at CT is not good enough. And
 the other part of the solution uses class just like std.logger.
Right, a CT LogLevel only works for certain use-cases and I never claimed that it works for everything (that's why there is the loggerObject adapter). This not an argument to go solely with classes though. And you loose an important optimization by not making use of CT LogLevels. Introducing the concepts and providing the loggerObject adapter (structs to interface) is a very flexible solution that proved itself very successful for ranges.
 And the hierarchy you're building is also at CT, which is just not gone
 work, if you don't have ultimate control of all sources.
What's the problem here, a counterexample would be helpful. Logger logger; if (config.alsoLogToFile) logger = loggerObject(multiLogger(SysLogger(), FileLogger())); else logger = loggerObject(SysLogger());
Oct 27 2014
prev sibling parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/26/2014 11:27 PM, Robert burner Schadek wrote:
 it is not really a dependency as the one template that uses the version
 identifier uses them optionally.
It simply doesn't work, e.g. you could not statically disable logging in phobos without recompiling phobos. cat > lib.d << CODE version (StdLoggerDisableLogging) enum isLoggingActive = false; else enum isLoggingActive = true; void doSome() { import std.stdio; writeln("isLoggingActive: ", isLoggingActive); } CODE cat > main.d << CODE import lib, std.stdio; void main() { writeln("isLoggingActive: ", isLoggingActive); doSome(); } CODE dmd -version=StdLoggerDisableLogging -lib lib.d dmd main lib.a ./main
Oct 26 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 26 October 2014 at 22:57:51 UTC, Martin Nowak wrote:
 On 10/26/2014 11:27 PM, Robert burner Schadek wrote:
 it is not really a dependency as the one template that uses 
 the version
 identifier uses them optionally.
It simply doesn't work, e.g. you could not statically disable logging in phobos without recompiling phobos. cat > lib.d << CODE version (StdLoggerDisableLogging) enum isLoggingActive = false; else enum isLoggingActive = true; void doSome() { import std.stdio; writeln("isLoggingActive: ", isLoggingActive); } CODE cat > main.d << CODE import lib, std.stdio; void main() { writeln("isLoggingActive: ", isLoggingActive); doSome(); } CODE dmd -version=StdLoggerDisableLogging -lib lib.d dmd main lib.a ./main
If it where done this way, yes of course you're right. But it is not, please take a look a the source first.
Oct 27 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 09:28 AM, Robert burner Schadek wrote:
 If it where done this way, yes of course you're right. But it is not,
 please take a look a the source first.
I'm looking at https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d and this is exactly how this works. Even if we make isLoggingActive a template, the problem persists. cat > lib.d << CODE version (StdLoggerDisableLogging) enum isLoggingActive() = false; else enum isLoggingActive() = true; void doSome() { import std.stdio; writeln("loggingLib: ", isLoggingActive!()); } CODE cat > main.d << CODE import lib, std.stdio; void main() { writeln("loggingMain: ", isLoggingActive!()); doSome(); } CODE dmd -version=StdLoggerDisableLogging -lib lib.d dmd main lib.a ./main ---- logginMain: true logginLib: false ---- dmd -lib lib.d dmd -version=StdLoggerDisableLogging main lib.a ./main ---- logginMain: false logginLib: true ----
Oct 27 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 27 October 2014 at 22:27:12 UTC, Martin Nowak wrote:
 On 10/27/2014 09:28 AM, Robert burner Schadek wrote:
 If it where done this way, yes of course you're right. But it 
 is not,
 please take a look a the source first.
I'm looking at https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d and this is exactly how this works.
take a look at https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L190 and https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L579 isLoggingActiveAt is instantiated at CT of the log function and the version statement inside the first link is evaluated at that moment. Disabling a version at CT of the lib has no consequence to compile units that are not compiled with that version statement. I tested it at https://github.com/burner/logger/blob/master/Makefile#L23 run "make info"
Oct 27 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/28/2014 12:58 AM, Robert burner Schadek wrote:
 Disabling a version at CT of the lib has no consequence to compile units
 that are not compiled with that version statement.
Yes setting a version in my app has no effect on the library, that's the problem because I cannot disable logging in a library. So I'll always have to pay some runtime overhead even though I don't want to use logging.
Oct 27 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 28 October 2014 at 01:37:48 UTC, Martin Nowak wrote:
 On 10/28/2014 12:58 AM, Robert burner Schadek wrote:
 Disabling a version at CT of the lib has no consequence to 
 compile units
 that are not compiled with that version statement.
Yes setting a version in my app has no effect on the library, that's the problem because I cannot disable logging in a library. So I'll always have to pay some runtime overhead even though I don't want to use logging.
Actually, that is only true for LogLevel given to a log call at runtime. calls to info, trace etc. are guarded with static if. So you're not paying any runtime overhead when calling log functions with LogLevel build in their name.
Oct 28 2014
parent reply "Martin Nowak" <code dawg.eu> writes:
On Tuesday, 28 October 2014 at 08:38:50 UTC, Robert burner
Schadek wrote:
 Actually, that is only true for LogLevel given to a log call at 
 runtime. calls to info, trace etc. are guarded with static if.
 So you're not paying any runtime overhead when calling log 
 functions with LogLevel build in their name.
If I cannot change the version identifiers that a library was compiled with then the static if is always true. So you have to perform the check at runtime. The only way around that is to make each library function that performs logging a template, be it by passing a Logger with CT LogLevel or by turning functions into template functions so that the version identifiers leak into the library. The latter is more fragile because the compiler might omit instatiating a template or it might inline a function.
Oct 28 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 28 October 2014 at 09:39:24 UTC, Martin Nowak wrote:
 On Tuesday, 28 October 2014 at 08:38:50 UTC, Robert burner
 Schadek wrote:
 Actually, that is only true for LogLevel given to a log call 
 at runtime. calls to info, trace etc. are guarded with static 
 if.
 So you're not paying any runtime overhead when calling log 
 functions with LogLevel build in their name.
If I cannot change the version identifiers that a library was compiled with then the static if is always true. So you have to perform the check at runtime. The only way around that is to make each library function that performs logging a template, be it by passing a Logger with CT LogLevel or by turning functions into template functions so that the version identifiers leak into the library.
every log call is a template function or method already.
 The latter is more fragile because the compiler might omit
 instatiating a template or it might inline a function.
omitting the instantiation sound like undefined reference. I don't see the problem with inlining. 1. there are no log functions only log templates. 2. The version statement will either leave empty template functions bodies, if(false) { ... } template functions bodies, or template function bodies that perform regular LogLevel checks on runtime LogLevel. We're running in circles here. Lets find a new solution that allows us to * dmd -version=StdLoggerDisableLogLevel -- disable a LogLevel at CT of user code * that does not require a static Logger LogLevel * does not leak version statements into phobos * does not call log template functions where the LogLevel of the version statement is part of the name, or at least yields empty bodies for these template functions callls
Oct 28 2014
parent reply "Martin Nowak" <code dawg.eu> writes:
On Tuesday, 28 October 2014 at 10:05:47 UTC, Robert burner 
Schadek wrote:
 On Tuesday, 28 October 2014 at 09:39:24 UTC, Martin Nowak wrote:
 On Tuesday, 28 October 2014 at 08:38:50 UTC, Robert burner
 Schadek wrote:
 Actually, that is only true for LogLevel given to a log call 
 at runtime. calls to info, trace etc. are guarded with static 
 if.
 So you're not paying any runtime overhead when calling log 
 functions with LogLevel build in their name.
If I cannot change the version identifiers that a library was compiled with then the static if is always true. So you have to perform the check at runtime. The only way around that is to make each library function that performs logging a template, be it by passing a Logger with CT LogLevel or by turning functions into template functions so that the version identifiers leak into the library.
every log call is a template function or method already.
 The latter is more fragile because the compiler might omit
 instatiating a template or it might inline a function.
omitting the instantiation sound like undefined reference. I don't see the problem with inlining. 1. there are no log functions only log templates. 2. The version statement will either leave empty template functions bodies, if(false) { ... } template functions bodies, or template function bodies that perform regular LogLevel checks on runtime LogLevel. We're running in circles here. Lets find a new solution that allows us to * dmd -version=StdLoggerDisableLogLevel -- disable a LogLevel at CT of user code * that does not require a static Logger LogLevel * does not leak version statements into phobos * does not call log template functions where the LogLevel of the version statement is part of the name, or at least yields empty bodies for these template functions callls
Yep, let's try that. I think part of the misunderstanding is that I'm thinking of an app as user code plus a number of libraries all on top of phobos. Say I have an app using vibe.d and I want to enable logging in my app, but disable it in phobos. Was it even a design goal that logging can be enabled on a per library level?
Oct 28 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 28 October 2014 at 11:11:09 UTC, Martin Nowak wrote:
 Yep, let's try that.

 I think part of the misunderstanding is that I'm thinking of an 
 app as user code plus a number of libraries all on top of 
 phobos. Say I have an app using vibe.d and I want to enable 
 logging in my app, but disable it in phobos.
 Was it even a design goal that logging can be enabled on a per 
 library level?
It is a design goal to disable certain LogLevel at CT of a compile unit (CU). e.g. make all logs to trace function template do nothing
Oct 28 2014
parent reply "Martin Nowak" <code dawg.eu> writes:
On Tuesday, 28 October 2014 at 12:02:16 UTC, Robert burner 
Schadek wrote:
 It is a design goal to disable certain LogLevel at CT of a 
 compile unit (CU).
 e.g. make all logs to trace function template do nothing
One idea to make this working is to use prefixed version identifiers. Obviously this is all boilerplate so it could be done in a mixin template in std.log. module mylib.log; version (MyLibLogInfo) enum logLevel = LogLevel.info; version (MyLibLogTrace) enum logLevel = LogLevel.trace; version (MyLibLogWarning) enum logLevel = LogLevel.warning; version (MyLibLogError) enum logLevel = LogLevel.error; static if (is(typeof(logLevel))) { // wrapper around Logger interface/class for CT logLevel static struct FixedLogLevel { enum logLevel = logLevel; Logger impl; alias impl this; } void setLogger(Logger logger) { logger.impl = logger; } private __gshared FixedLogLevel logger; } module mylib.foo; void foo() { import mylib.log; logger.info("bla"); logger.error("wat"); } module client.bar; void bar() { import mylib.log, mylib.foo; auto logger = new FileLogger("log.txt", LogLevel.warning); setLogger(logger); foo(); // CT check for LogLevel logger.info("runtime check whether info logging is on"); }
Oct 28 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/28/2014 07:22 PM, Martin Nowak wrote:
 On Tuesday, 28 October 2014 at 12:02:16 UTC, Robert burner Schadek wrote:
 It is a design goal to disable certain LogLevel at CT of a compile
 unit (CU).
 e.g. make all logs to trace function template do nothing
One idea to make this working is to use prefixed version identifiers. Obviously this is all boilerplate so it could be done in a mixin template in std.log.
2nd iteration of that idea. cat > main.d << CODE import std_logger; LoggerCT!"MyApp" appLogger = new StdoutLogger(LogLevel.info); LoggerCT!"MyLib" libLogger = new StdoutLogger(LogLevel.trace); void main() { appLogger.log!(LogLevel.info)("app"); appLogger.log!(LogLevel.warning)("app"); libLogger.log!(LogLevel.info)("lib"); libLogger.log!(LogLevel.warning)("lib"); Logger rtLogger = appLogger; rtLogger.log!(LogLevel.info)("rt"); rtLogger.log!(LogLevel.warning)("rt"); } CODE cat > std_logger.d << CODE enum LogLevel { all, info, trace, warning, error, none } template minLogLevel(string prefix) { mixin(" version ("~prefix~"LogAll) enum minLogLevel = LogLevel.all; else version ("~prefix~"LogInfo) enum minLogLevel = LogLevel.info; else version ("~prefix~"LogTrace) enum minLogLevel = LogLevel.trace; else version ("~prefix~"LogWarning) enum minLogLevel = LogLevel.warning; else version ("~prefix~"LogError) enum minLogLevel = LogLevel.error; else version ("~prefix~"LogNone) enum minLogLevel = LogLevel.none; else enum minLogLevel = LogLevel.all; "); } interface Logger { property LogLevel logLevel(); void write(LogLevel ll, string msg); } class StdoutLogger : Logger { this(LogLevel l) { _logLevel = l; } LogLevel _logLevel; property LogLevel logLevel() { return _logLevel; } void write(LogLevel ll, string msg) { import std.stdio; writeln(ll, ": ", msg); } } /// used for library/app specific version prefixes struct LoggerCT(string prefix) { this(Logger logger) { impl = logger; } enum versionPrefix = prefix; Logger impl; alias impl this; } void log(LogLevel ll)(Logger logger, string msg) { if (ll >= logger.logLevel) // runtime check logger.write(ll, msg); } /// when using a logger with prefix void log(LogLevel ll, L:LoggerCT!pfx, string pfx)(auto ref L logger, string msg) if (ll < minLogLevel!pfx) { } void log(LogLevel ll, L:LoggerCT!pfx, string pfx)(auto ref L logger, string msg) if (ll >= minLogLevel!pfx) { if (ll >= logger.logLevel) // additional runtime check, because logger might require a higher log level logger.write(ll, msg); } CODE dmd std_logger -run main dmd -version=MyAppLogWarning std_logger.d -run main dmd -version=MyAppLogError -version=MyLibLogNone std_logger.d -run main
Oct 28 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 28 October 2014 at 22:03:18 UTC, Martin Nowak wrote:
 On 10/28/2014 07:22 PM, Martin Nowak wrote:
 On Tuesday, 28 October 2014 at 12:02:16 UTC, Robert burner 
 Schadek wrote:
 It is a design goal to disable certain LogLevel at CT of a 
 compile
 unit (CU).
 e.g. make all logs to trace function template do nothing
One idea to make this working is to use prefixed version identifiers. Obviously this is all boilerplate so it could be done in a mixin template in std.log.
2nd iteration of that idea. cat > main.d << CODE import std_logger; LoggerCT!"MyApp" appLogger = new StdoutLogger(LogLevel.info); LoggerCT!"MyLib" libLogger = new StdoutLogger(LogLevel.trace); void main() { appLogger.log!(LogLevel.info)("app"); appLogger.log!(LogLevel.warning)("app"); libLogger.log!(LogLevel.info)("lib"); libLogger.log!(LogLevel.warning)("lib"); Logger rtLogger = appLogger; rtLogger.log!(LogLevel.info)("rt"); rtLogger.log!(LogLevel.warning)("rt"); } CODE cat > std_logger.d << CODE enum LogLevel { all, info, trace, warning, error, none } template minLogLevel(string prefix) { mixin(" version ("~prefix~"LogAll) enum minLogLevel = LogLevel.all; else version ("~prefix~"LogInfo) enum minLogLevel = LogLevel.info; else version ("~prefix~"LogTrace) enum minLogLevel = LogLevel.trace; else version ("~prefix~"LogWarning) enum minLogLevel = LogLevel.warning; else version ("~prefix~"LogError) enum minLogLevel = LogLevel.error; else version ("~prefix~"LogNone) enum minLogLevel = LogLevel.none; else enum minLogLevel = LogLevel.all; "); } interface Logger { property LogLevel logLevel(); void write(LogLevel ll, string msg); } class StdoutLogger : Logger { this(LogLevel l) { _logLevel = l; } LogLevel _logLevel; property LogLevel logLevel() { return _logLevel; } void write(LogLevel ll, string msg) { import std.stdio; writeln(ll, ": ", msg); } } /// used for library/app specific version prefixes struct LoggerCT(string prefix) { this(Logger logger) { impl = logger; } enum versionPrefix = prefix; Logger impl; alias impl this; } void log(LogLevel ll)(Logger logger, string msg) { if (ll >= logger.logLevel) // runtime check logger.write(ll, msg); } /// when using a logger with prefix void log(LogLevel ll, L:LoggerCT!pfx, string pfx)(auto ref L logger, string msg) if (ll < minLogLevel!pfx) { } void log(LogLevel ll, L:LoggerCT!pfx, string pfx)(auto ref L logger, string msg) if (ll >= minLogLevel!pfx) { if (ll >= logger.logLevel) // additional runtime check, because logger might require a higher log level logger.write(ll, msg); } CODE dmd std_logger -run main dmd -version=MyAppLogWarning std_logger.d -run main dmd -version=MyAppLogError -version=MyLibLogNone std_logger.d -run main
That can actually be added to the current state of std.logger without breaking any api. The string mixin, version string matching isn't really pretty, but it gets the job done. Anyway IMO your approach presented here and my approach can go hand in hang. Yours should be propagated as the idiomatic way and if you really need the crowbar, which you need sometimes, use StdLoggerDisableXXXXX.
Oct 29 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
The reason for the crowbar sometimes you need to disable all 
calls to the Logger or any calls to a specific LogLevel in the 
compile unit, even for Logger not wrapped in LoggerCT.
Oct 29 2014
parent "Martin Nowak" <code dawg.eu> writes:
On Wednesday, 29 October 2014 at 23:16:28 UTC, Robert burner 
Schadek wrote:
 The reason for the crowbar sometimes you need to disable all 
 calls to the Logger or any calls to a specific LogLevel in the 
 compile unit, even for Logger not wrapped in LoggerCT.
Only that there is no clean boarder between modules because of inlining and templates.
Nov 16 2014
prev sibling next sibling parent "Martin Nowak" <code dawg.eu> writes:
On Wednesday, 29 October 2014 at 23:11:44 UTC, Robert burner 
Schadek wrote:
 That can actually be added to the current state of std.logger 
 without breaking any api.
Just a few additions to handle LoggerCT (which needs a better name).
 Anyway IMO your approach presented here and my approach can go 
 hand in hang. Yours should be propagated as the idiomatic way 
 and if you really need the crowbar, which you need sometimes, 
 use StdLoggerDisableXXXXX.
Yes, that would work for me. It's just important to control logging on a per library basis.
Nov 16 2014
prev sibling parent Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/30/2014 12:11 AM, Robert burner Schadek wrote:
 That can actually be added to the current state of std.logger without
 breaking any api. The string mixin, version string matching isn't really
 pretty, but it gets the job done. Anyway IMO your approach presented
 here and my approach can go hand in hang. Yours should be propagated as
 the idiomatic way and if you really need the crowbar, which you need
 sometimes, use StdLoggerDisableXXXXX.
I just found a very compelling solution to the LogLevel disabling problem. Basically we can declare a logLevel for each module or package and let the logger do a reverse look up. ```d module mymod; import logger; // can be anything that converts to a LogLevel // calls with lower log level can be optimized away if this is a compile time constant // runtime values also work nicely but incur a small overhead // if this declaration is not present, logLevel from the package is used or a default LogLevel. enum logLevel = LogLevel.critical; void foo() { info("information from foo"); // optimized out fatal("error"); // works } ``` https://gist.github.com/MartinNowak/443f11aa017d14007c35 There is a tiny gotcha, this works because the logger module imports the callee module. So we'd need to avoid module constructors in the logger module or use a workaround.
Dec 03 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Tuesday, 28 October 2014 at 22:03:18 UTC, Martin Nowak wrote:
 2nd iteration of that idea.
For me it looks like the cure is worse than the disease. Simply not distributing precompiled libraries with log level force-reduced (or at least not exposing it in provided .di files) feels like a more practical approach than proposed API complication.
Oct 31 2014
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
Bringing this back to the first page
Nov 01 2014
parent reply "David Nadlinger" <code klickverbot.at> writes:
There is still a critical issue with std.experimental.logger that 
would prevent it from being merged right now: The abuse of 
 trusted all over the code.

For example, the following piece of code compiles with 
burner/logger c87e1032:
---
import std.experimental.logger.core;

struct Dangerous {
     string toString() {
         *(cast(int*)0xdeadbeef) = 0xcafebabe;
         return null;
     }
}

void main()  safe {
     logf("%s", Dangerous());
}
---

To be quite honest, I'm rather disappointed that I was able to 
break  safe-ty
with the first, most straightforward piece of code I tried. Sure, 
many people don't understand how dangerous  trusted applied to 
templates really is. But this issue has already been pointed out 
multiple times in Robert's code, both in this pull request and 
other unrelated ones.

David
Nov 01 2014
next sibling parent "Dicebot" <public dicebot.lv> writes:
On Saturday, 1 November 2014 at 17:35:37 UTC, David Nadlinger 
wrote:
 There is still a critical issue with std.experimental.logger 
 that would prevent it from being merged right now: The abuse of 
  trusted all over the code.

 For example, the following piece of code compiles with 
 burner/logger c87e1032:
 ---
 import std.experimental.logger.core;

 struct Dangerous {
     string toString() {
         *(cast(int*)0xdeadbeef) = 0xcafebabe;
         return null;
     }
 }

 void main()  safe {
     logf("%s", Dangerous());
 }
 ---

 To be quite honest, I'm rather disappointed that I was able to 
 break  safe-ty
 with the first, most straightforward piece of code I tried. 
 Sure, many people don't understand how dangerous  trusted 
 applied to templates really is. But this issue has already been 
 pointed out multiple times in Robert's code, both in this pull 
 request and other unrelated ones.

 David
Oh, I have actually completely missed that. It is critical indeed. Will try having a look myself and possibly providing patches.
Nov 01 2014
prev sibling next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Saturday, 1 November 2014 at 17:35:37 UTC, David Nadlinger 
wrote:
 There is still a critical issue with std.experimental.logger 
 that would prevent it from being merged right now: The abuse of 
  trusted all over the code.
Thank you. I was afraid I'd have to harp on it for the fourth time...
Nov 01 2014
parent "Dicebot" <public dicebot.lv> writes:
On Sunday, 2 November 2014 at 00:29:10 UTC, Jakob Ovrum wrote:
 On Saturday, 1 November 2014 at 17:35:37 UTC, David Nadlinger 
 wrote:
 There is still a critical issue with std.experimental.logger 
 that would prevent it from being merged right now: The abuse 
 of  trusted all over the code.
Thank you. I was afraid I'd have to harp on it for the fourth time...
Sorry, it is really hard to follow important bits in many discussions spawned :( I'll take a go at safe-ty issue personally this time.
Nov 02 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
I had initial go at cleaning safety attributes : 
https://github.com/Dicebot/phobos/tree/logger-fix-trust

It still needs one druntime PR merged 
(https://github.com/D-Programming-Language/druntime/pull/1009) to 
be done properly but should be ready for destruction.

David snipet results in this after all changes:
std/experimental/logger/core.d(1675): Error: safe function 
'std.experimental.logger.core.Logger.logf!(11, "aaa.d", 
"aaa.main", "void aaa.main()  safe", "aaa", Dangerous).logf' 
cannot call system function 'std.format.formattedWrite!(MsgRange, 
char, Dangerous).formattedWrite'
Nov 02 2014
parent reply "David Nadlinger" <code klickverbot.at> writes:
On Sunday, 2 November 2014 at 13:06:24 UTC, Dicebot wrote:
 I had initial go at cleaning safety attributes : 
 https://github.com/Dicebot/phobos/tree/logger-fix-trust

 It still needs one druntime PR merged 
 (https://github.com/D-Programming-Language/druntime/pull/1009) 
 to be done properly but should be ready for destruction.

 David snipet results in this after all changes:
 std/experimental/logger/core.d(1675): Error: safe function 
 'std.experimental.logger.core.Logger.logf!(11, "aaa.d", 
 "aaa.main", "void aaa.main()  safe", "aaa", Dangerous).logf' 
 cannot call system function 
 'std.format.formattedWrite!(MsgRange, char, 
 Dangerous).formattedWrite'
I don't recall off the top of my head some non-template innards actually might require safe, but apart from that, why not just leave the job to template attribute inference entirely? If somebody wants to log a type with a system toString in non- safe code, why not just let them? David
Nov 02 2014
parent reply "Dicebot" <public dicebot.lv> writes:
 I don't recall off the top of my head some non-template innards 
 actually might require  safe, but apart from that, why not just 
 leave the job to template attribute inference entirely? If 
 somebody wants to log a type with a  system toString in 
 non- safe code, why not just let them?

 David
In my opinion inference is better choice for small building blocks (like algorithms). For complete system like logging API forcing safe makes more sense as whatever its internals are, exposed API should never be system
Nov 02 2014
parent reply "David Nadlinger" <code klickverbot.at> writes:
On Sunday, 2 November 2014 at 18:29:09 UTC, Dicebot wrote:
 In my opinion inference is better choice for small building 
 blocks (like algorithms). For complete system like logging API 
 forcing  safe makes more sense as whatever its internals are, 
 exposed API should never be  system
This isn't about the library internals. These should be presented in a safe way, of course. The point here is that you restrict what your users can do by forcing templates to be safe. Imagine somebody has a type that cannot be trusted because of whatever reason. Maybe because it's legacy code, maybe it uses resources it does not manage, … If you forcibly make logf safe, then this type cannot be used with logf without some crazy workaround (simply using to!string might produce an unneeded allocation if the type uses the sink-delegate signature for toString). Why not leave this up to the compiler and support more use cases without degrading the experience for safe clients? David
Nov 02 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 2 November 2014 at 18:42:20 UTC, David Nadlinger wrote:
 Imagine somebody has a type that cannot be  trusted because of 
 whatever reason. Maybe because it's legacy code, maybe it uses 
 resources it does not manage, … If you forcibly make logf 
  safe, then this type cannot be used with logf without some 
 crazy workaround (simply using to!string might produce an 
 unneeded allocation if the type uses the sink-delegate 
 signature for toString).

 Why not leave this up to the compiler and support more use 
 cases without degrading the experience for  safe clients?

 David
You mean something like user type toString() which is legitimately system and can't be made trusted? Yes, this makes sense. Will need to also add tests for that. Consider me convinced :)
Nov 02 2014
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/2/14 10:07 PM, Dicebot wrote:
 On Sunday, 2 November 2014 at 18:42:20 UTC, David Nadlinger wrote:
 Imagine somebody has a type that cannot be  trusted because of
 whatever reason. Maybe because it's legacy code, maybe it uses
 resources it does not manage, … If you forcibly make logf  safe, then
 this type cannot be used with logf without some crazy workaround
 (simply using to!string might produce an unneeded allocation if the
 type uses the sink-delegate signature for toString).

 Why not leave this up to the compiler and support more use cases
 without degrading the experience for  safe clients?

 David
You mean something like user type toString() which is legitimately system and can't be made trusted? Yes, this makes sense. Will need to also add tests for that. Consider me convinced :)
Fantastic, thanks! -- Andrei
Nov 03 2014
prev sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
I will remove the trusted later this week
Nov 03 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 3 November 2014 at 20:41:14 UTC, Robert burner Schadek 
wrote:
 I will remove the trusted later this week
After few tweaks (to allows system toString()) in my branch you should be able to just merge it directly to your PR branch if you are ok with proposed changes.
Nov 03 2014
parent reply "Dicebot" <public dicebot.lv> writes:
And back to the frontpage.

Martin / Robert, have you managed to come to an agreement on 
conditional level thing?
Nov 09 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 9 November 2014 at 21:42:09 UTC, Dicebot wrote:
 And back to the frontpage.

 Martin / Robert, have you managed to come to an agreement on 
 conditional level thing?
After your last reply we haven't done anything else. I would merge his idea, but there is pushback from your side. Anyway, this idea would not break the api. So I don't think it is a blocking change, IMO.
Nov 10 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 10 November 2014 at 11:46:34 UTC, Robert burner 
Schadek wrote:
 After your last reply we haven't done anything else.

 I would merge his idea, but there is pushback from your side. 
 Anyway, this idea would not break the api. So I don't think it 
 is a blocking change, IMO.
kk, I'll try contacting him via e-mail if Martin does not return to this thread soon enough :) This proposal may not break API directly but it greatly affects how derivative libraries are supposed to be designed and distributed so I'd prefer to nail it down immediately. There are also some more safe changes need, I hope to provide another branch with those soon enough (~tomorrow)
Nov 10 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 10 November 2014 at 17:03:31 UTC, Dicebot wrote:
 On Monday, 10 November 2014 at 11:46:34 UTC, Robert burner 
 Schadek wrote:
 After your last reply we haven't done anything else.

 I would merge his idea, but there is pushback from your side. 
 Anyway, this idea would not break the api. So I don't think it 
 is a blocking change, IMO.
kk, I'll try contacting him via e-mail if Martin does not return to this thread soon enough :) This proposal may not break API directly but it greatly affects how derivative libraries are supposed to be designed and distributed so I'd prefer to nail it down immediately.
I get that
 There are also some more  safe changes need, I hope to provide 
 another branch with those soon enough (~tomorrow)
thank you
Nov 10 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 10 November 2014 at 18:32:03 UTC, Robert burner 
Schadek wrote:
 On Monday, 10 November 2014 at 17:03:31 UTC, Dicebot wrote:
 There are also some more  safe changes need, I hope to provide 
 another branch with those soon enough (~tomorrow)
thank you
https://github.com/Dicebot/phobos/tree/logger-safety I think that should be it for now
Nov 11 2014
parent reply "Jose" <jsancio gmail.com> writes:
On Tuesday, 11 November 2014 at 15:06:49 UTC, Dicebot wrote:
 https://github.com/Dicebot/phobos/tree/logger-safety

 I think that should be it for now
I could have completely missed some details but I took sometime to look at the code after reading that the logger was holding a lock during the write operation. I noticed the following lines. One shared Logger: https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L1696 One global function that references that shared Logger: https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L292 And more importantly a Mutex that is always acquired when writing: https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L1035 Does this mean that if I use this library on a machine that has 32 cores, 100s of threads and 1000s of fiber only one thread/fiber can write at a time no matter the concrete implementation of my logger? Am I missing something or isn't this a non-starter? Thanks, -Jose
Nov 11 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Wednesday, 12 November 2014 at 05:36:40 UTC, Jose wrote:
 On Tuesday, 11 November 2014 at 15:06:49 UTC, Dicebot wrote:
 https://github.com/Dicebot/phobos/tree/logger-safety
One shared Logger: https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L1696 One global function that references that shared Logger: https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L292 And more importantly a Mutex that is always acquired when writing: https://github.com/burner/phobos/blob/logger/std/experimental/logger/core.d#L1035 Does this mean that if I use this library on a machine that has 32 cores, 100s of threads and 1000s of fiber only one thread/fiber can write at a time no matter the concrete implementation of my logger? Am I missing something or isn't this a non-starter? Thanks, -Jose
Only one thread can write to one Logger at a time, also known as synchronization. Anything else is properly wrong. But you can have as many Logger as you have memory.
Nov 12 2014
next sibling parent reply "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Wednesday, 12 November 2014 at 12:39:24 UTC, Robert burner 
Schadek wrote:
 Only one thread can write to one Logger at a time, also known 
 as synchronization. Anything else is properly wrong. But you 
 can have as many Logger as you have memory.
Taking a lock when the logging call doesn't flush to disk sounds rather expensive.
Nov 12 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Wednesday, 12 November 2014 at 15:05:29 UTC, Ola Fosheim 
Grøstad wrote:
 On Wednesday, 12 November 2014 at 12:39:24 UTC, Robert burner 
 Schadek wrote:
 Only one thread can write to one Logger at a time, also known 
 as synchronization. Anything else is properly wrong. But you 
 can have as many Logger as you have memory.
Taking a lock when the logging call doesn't flush to disk sounds rather expensive.
If you log to a FileLogger it will flush. It is just a costly thing to do. There is no way get the work done but doing it.
Nov 13 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 12 November 2014 at 12:39:24 UTC, Robert burner 
Schadek wrote:
 Only one thread can write to one Logger at a time, also known 
 as synchronization. Anything else is properly wrong. But you 
 can have as many Logger as you have memory.
One thing we can improve is to use thread-local stdlog singletons (that forward to global shared one by default) instead of using shared one as entry point. In server applications one would typically want to have logs buffered per thread or even sent to remote process / machine in parallel - while this can be done right now it would require applications to resort from using stdlog completely. Better approach that comes to my mind is to have both thread-local and global configurable stdlog and have means to explicitly capture lock on global one from local proxies for optimized bulk logging.
Nov 12 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
IMO this defeats the design goal off having the default case very 
easy and just working. Therefore, I think thread local global 
Logger and how to make them interact is something that should be 
left to the advanced user.
Nov 13 2014
next sibling parent "Dicebot" <public dicebot.lv> writes:
On Thursday, 13 November 2014 at 19:59:21 UTC, Robert burner 
Schadek wrote:
 IMO this defeats the design goal off having the default case 
 very easy and just working. Therefore, I think thread local 
 global Logger and how to make them interact is something that 
 should be left to the advanced user.
How so? For casual end user hardly anything changes. Right now there is a single shared stdlog singleton (configurable). I propose to rename it to something like stdlog_shared and provide thread-local stdlog singletons which by default will simply forward calls to stdlog_shared. That way default case will work exactly like it does now but it will make possible for advanced user to implement bulk loggers while still using same `stdlog` entry point (which is crucial in my opinion)
Nov 14 2014
prev sibling parent reply "David Nadlinger" <code klickverbot.at> writes:
On Thursday, 13 November 2014 at 19:59:21 UTC, Robert burner 
Schadek wrote:
 Therefore, I think thread local global Logger and how to make 
 them interact is something that should be left to the advanced 
 user.
Except that they can't actually get rid of all the overhead involved, as the locking is hard-coded into the Logger base-class. Granted, acquiring an uncontended lock isn't terribly expensive, but it's still a noticeable overhead if you are just logging to a thread-local buffer otherwise. David
Nov 14 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 14 November 2014 at 21:43:53 UTC, David Nadlinger 
wrote:
 Except that they can't actually get rid of all the overhead 
 involved, as the locking is hard-coded into the Logger 
 base-class. Granted, acquiring an uncontended lock isn't 
 terribly expensive, but it's still a noticeable overhead if you 
 are just logging to a thread-local buffer otherwise.

 David
You can always roll your own non locking Logger, but the default should be thread-safe.
Nov 14 2014
parent reply "David Nadlinger" <code klickverbot.at> writes:
On Friday, 14 November 2014 at 21:46:00 UTC, Robert burner 
Schadek wrote:
 You can always roll your own non locking Logger, but the 
 default should be thread-safe.
You can't, since you need to inherit from Logger, which already does the locking for you in a way that's not overridable. Also, I take it you are familiar with the fact that locks aren't the only technique for achieving cross-thread synchronization. David
Nov 14 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 14 November 2014 at 21:49:09 UTC, David Nadlinger 
wrote:
 On Friday, 14 November 2014 at 21:46:00 UTC, Robert burner 
 Schadek wrote:
 You can always roll your own non locking Logger, but the 
 default should be thread-safe.
You can't, since you need to inherit from Logger, which already does the locking for you in a way that's not overridable. Also,
My bad, I mixed something up.
 I take it you are familiar with the fact that locks aren't the 
 only technique for achieving cross-thread synchronization.
Yes, but this way allows to add structured logging in an easy way.
Nov 14 2014
parent reply "David Nadlinger" <code klickverbot.at> writes:
On Friday, 14 November 2014 at 22:18:41 UTC, Robert burner 
Schadek wrote:
 Yes, but this way allows to add structured logging in an easy 
 way.
What.
Nov 14 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 14 November 2014 at 23:40:21 UTC, David Nadlinger 
wrote:
 On Friday, 14 November 2014 at 22:18:41 UTC, Robert burner 
 Schadek wrote:
 Yes, but this way allows to add structured logging in an easy 
 way.
What.
The actual log call is split into multiple parts so that the user can overload logMsgPart and add, for instance, structured logging. In order to not have races with the default impl locking was added.
Nov 14 2014
parent Marco Leise <Marco.Leise gmx.de> writes:
Am Sat, 15 Nov 2014 02:11:34 +0000
schrieb "Robert burner Schadek" <rburners gmail.com>:

 On Friday, 14 November 2014 at 23:40:21 UTC, David Nadlinger 
 wrote:
 On Friday, 14 November 2014 at 22:18:41 UTC, Robert burner 
 Schadek wrote:
 Yes, but this way allows to add structured logging in an easy 
 way.
What.
The actual log call is split into multiple parts so that the user can overload logMsgPart and add, for instance, structured logging. In order to not have races with the default impl locking was added.
Somehow I feel like I should be in the firing line here, hehe. Yes, this is approach to thread-safety isn't the most efficient imaginable. But it can guarantee thread-safety no matter how little or much you know about the default logger internals or what you plan to do with it. The lock also ensures that public properties like logLevel cannot change while a logger is working, which could result in log messages being dropped that should have passed and - as Robert mentioned - ensures that the 3 structural logging methods are called in order with no other thread intervening. I believe it will hit a sweet spot between safe extensibility and performance. Benchmarking... Hmm, you can take an uncontested lock ~14.000.000 times per second on Linux 2 Ghz with D's synchronized(...) {} statement and you can append "The quick brown fox jumps over the lazy dog." ~22.400.000 to an Appender!(string[]). I begin to understand what you mean by "noticeable". Somehow I hope that people don't log that much or that the loggers themselves are sufficiently complex that they take their time to process each message. But for a thread-local logger that just appends to a list, it is quite a bit of unnecessary overhead. -- Marco
Nov 18 2014
prev sibling parent reply "Martin Nowak" <code dawg.eu> writes:
On Friday, 14 November 2014 at 21:49:09 UTC, David Nadlinger 
wrote:
 On Friday, 14 November 2014 at 21:46:00 UTC, Robert burner 
 Schadek wrote:
 You can always roll your own non locking Logger, but the 
 default should be thread-safe.
You can't, since you need to inherit from Logger, which already does the locking for you in a way that's not overridable. Also, I take it you are familiar with the fact that locks aren't the only technique for achieving cross-thread synchronization.
Have you guys resolved this? If we cannot implement a non-locking logger than that's a blocking issue for the library.
Nov 29 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 29 November 2014 at 14:15:12 UTC, Martin Nowak wrote:
 On Friday, 14 November 2014 at 21:49:09 UTC, David Nadlinger 
 wrote:
 On Friday, 14 November 2014 at 21:46:00 UTC, Robert burner 
 Schadek wrote:
 You can always roll your own non locking Logger, but the 
 default should be thread-safe.
You can't, since you need to inherit from Logger, which already does the locking for you in a way that's not overridable. Also, I take it you are familiar with the fact that locks aren't the only technique for achieving cross-thread synchronization.
Have you guys resolved this? If we cannot implement a non-locking logger than that's a blocking issue for the library.
Yes, there is a lock free, thread local indirection now. That can be used to build a lock free, thread local logger. p.s. You should have taken the phun
Nov 29 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 11/29/2014 05:25 PM, Robert burner Schadek wrote:
 Yes, there is a lock free, thread local indirection now. That can be
 used to build a lock free, thread local logger.

 p.s. You should have taken the phun
I commented on the relevant commit. https://github.com/burner/phobos/commit/8a3aad5df5218bd995d4679f9b59a59909969b52#diff-59d32a64bcbd4492ff85f10091d73f5fR289 Let me propose a light-weight alternative solution. ```d abstract class Logger { Object.Monitor mutex; this() { this.mutex = createMutex(); } Object.Monitor createMutex() { import core.sync.mutex; return new Mutex; } } ``` This allows people to override the mutex with whatever fits their bill, e.g. one that supports fiber suspension. Especially they can do this. Object.Monitor createMutex() { static class NoLock : Object.Monitor { void lock() nothrow {} void unlock() nothrow {} } return new NoLock; }
Nov 29 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Saturday, 29 November 2014 at 19:18:01 UTC, Martin Nowak wrote:
 On 11/29/2014 05:25 PM, Robert burner Schadek wrote:
 Yes, there is a lock free, thread local indirection now. That 
 can be
 used to build a lock free, thread local logger.

 p.s. You should have taken the phun
I commented on the relevant commit. https://github.com/burner/phobos/commit/8a3aad5df5218bd995d4679f9b59a59909969b52#diff-59d32a64bcbd4492ff85f10091d73f5fR289
I missed that final.
 Let me propose a light-weight alternative solution.

 ```d
 abstract class Logger
 {
     Object.Monitor mutex;

     this()
     {
         this.mutex = createMutex();
     }

     Object.Monitor createMutex()
     {
         import core.sync.mutex;
         return new Mutex;
     }
 }
 ```

 This allows people to override the mutex with whatever fits 
 their bill, e.g. one that supports fiber suspension. Especially 
 they can do this.

 Object.Monitor createMutex()
 {
     static class NoLock : Object.Monitor
     {
         void lock() nothrow {}
         void unlock() nothrow {}
     }
     return new NoLock;
 }
I will add this and remove the final. Thank you
Nov 30 2014
prev sibling parent reply Marco Leise <Marco.Leise gmx.de> writes:
Am Wed, 12 Nov 2014 17:56:06 +0000
schrieb "Dicebot" <public dicebot.lv>:

 [=E2=80=A6] have means to=20
 explicitly capture lock on global one from local proxies for=20
 optimized bulk logging.
That's certainly something that occurred to me when talking with Manu about logging. A locked bulk transfer could be added, maybe replacing the single element transfer function that exists already. Thread local loggers that accumulate logs and pass them on to a global logger seem to be a well known use case. --=20 Marco
Nov 13 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Thursday, 13 November 2014 at 22:13:53 UTC, Marco Leise wrote:
 Am Wed, 12 Nov 2014 17:56:06 +0000
 schrieb "Dicebot" <public dicebot.lv>:

 […] have means to explicitly capture lock on global one from 
 local proxies for optimized bulk logging.
That's certainly something that occurred to me when talking with Manu about logging. A locked bulk transfer could be added, maybe replacing the single element transfer function that exists already. Thread local loggers that accumulate logs and pass them on to a global logger seem to be a well known use case.
One bad thing about that is that the global log is no longer sorted by time if you write to any file. That would make using the log much more difficult IMO.
Nov 13 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 13 November 2014 at 22:19:55 UTC, Robert burner
Schadek wrote:
 One bad thing about that is that the global log is no longer 
 sorted by time if you write to any file. That would make using 
 the log much more difficult IMO.
We do have timestamp as part of recorded log data, don't we?
Nov 14 2014
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 11/14/14 4:22 PM, Dicebot wrote:
 On Thursday, 13 November 2014 at 22:19:55 UTC, Robert burner
 Schadek wrote:
 One bad thing about that is that the global log is no longer sorted by
 time if you write to any file. That would make using the log much more
 difficult IMO.
We do have timestamp as part of recorded log data, don't we?
I'm not following this thread, but please please please -- output log data in chronological order. If you have to have another object that does it, fine. But I don't want to have to rely on sorting utilities to properly read a log file. -Steve
Nov 14 2014
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Friday, 14 November 2014 at 22:02:30 UTC, Steven Schveighoffer 
wrote:
 On 11/14/14 4:22 PM, Dicebot wrote:
 On Thursday, 13 November 2014 at 22:19:55 UTC, Robert burner
 Schadek wrote:
 One bad thing about that is that the global log is no longer 
 sorted by
 time if you write to any file. That would make using the log 
 much more
 difficult IMO.
We do have timestamp as part of recorded log data, don't we?
I'm not following this thread, but please please please -- output log data in chronological order. If you have to have another object that does it, fine. But I don't want to have to rely on sorting utilities to properly read a log file. -Steve
I have meant that shared logger can do sorting based on timestamp upon receiving chunks for thread-local ones before sending it forward - it would require defining strict syncing period between loggers though.
Nov 14 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
I will test something this weekend regarding the additional 
indirection.
Nov 14 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Friday, 14 November 2014 at 22:20:17 UTC, Robert burner 
Schadek wrote:
 I will test something this weekend regarding the additional 
 indirection.
Thanks! I may try hacking some sample implementation too but pessimistic about ETA
Nov 14 2014
parent "Robert burner Schadek" <rburners gmail.com> writes:
On Friday, 14 November 2014 at 23:06:22 UTC, Dicebot wrote:
 On Friday, 14 November 2014 at 22:20:17 UTC, Robert burner 
 Schadek wrote:
 I will test something this weekend regarding the additional 
 indirection.
Thanks! I may try hacking some sample implementation too but pessimistic about ETA
So, I added a layer of thread local indirection to the Logger. It now goes by default like: log|trace|... -> threadLocalLogger -> globalLogger The threadLocalLogger is just another Logger so it can be replaced with whatever you need may need. Or just keep it forwarding.
Nov 24 2014
prev sibling parent "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Friday, 14 November 2014 at 22:02:30 UTC, Steven Schveighoffer 
wrote:
 I'm not following this thread, but please please please -- 
 output log data in chronological order.
I'm not sure if you can define a strict chronological order between threads since logging happens "after the fact" which may be truly parallel (in a race). If you want strict chronological order between threads you can use an atomic counter for a serial id… For a web server this is usually not that interesting. You are interested in chronological order on the first dispatch and then on the request (the fiber id?)... When you are doing buffering with one producer and one consumer on a circular buffer you usually don't need locking or atomics since single read/writes are atomic on x86 AFAIK. And with double/triple buffering you only need an occasional CAS. I can think of at least of 3-4 ways of getting a performant multithreaded logger with almost no locking.
Nov 14 2014
prev sibling parent "Martin Nowak" <code dawg.eu> writes:
On Friday, 31 October 2014 at 15:35:55 UTC, Dicebot wrote:
 On Tuesday, 28 October 2014 at 22:03:18 UTC, Martin Nowak wrote:
 2nd iteration of that idea.
For me it looks like the cure is worse than the disease. Simply not distributing precompiled libraries with log level force-reduced (or at least not exposing it in provided .di files) feels like a more practical approach than proposed API complication.
There are no .di files in a dub package. And for certain libraries the runtime overhead for checking log levels isn't acceptable, so we need a non-global way to configure logging of a library.
Nov 16 2014
prev sibling parent "Kevin Lamonte" <kevindotlamnodotonte gmail.com> writes:
On Tuesday, 28 October 2014 at 11:11:09 UTC, Martin Nowak wrote:
 I think part of the misunderstanding is that I'm thinking of an 
 app as user code plus a number of libraries all on top of 
 phobos. Say I have an app using vibe.d and I want to enable 
 logging in my app, but disable it in phobos.
 Was it even a design goal that logging can be enabled on a per 
 library level?
Whether it was an original design goal or not, I don't think it can be done with today's std.logger, and I would argue that it should not be a function of std.logger at all. Selective logging of a mix of libraries requires a namespace to identify each library, along with a method for libraries to obtain their Loggers based on that namespace (e.g. giving Logger a name, which it does not have now, and using a factory method like Logger.getLogger()). We could assume that module names will always be available to do that job (and by default I do provide that as a reasonable default in log4d), but I can easily imagine other namespace choices for unknown future users. I also think that most end users would prefer their selective enabling/disabling of libraries' logging to happen at runtime, not compile time, and the HOW of turning things on/off will also be very user-dependent (regex, config file, environment var, ...). We are bleeding between "mechanism" and "policy" here. I feel that std.logger is mechanism: given that a client has a Logger "thing" (struct, class, whatever), which methods will the client call on it to emit output to somewhere? Frameworks like log4d are policy: how are the Loggers that are obtained by lots of clients organized into a coherent whole, and the outputs filtered/directed/controlled by the final end-user? Policy should always be deferred to third party libraries. I obviously like the log4d/log4j approach, but other people will like vibe.d logger, some will roll-your-own, some will just send everything to syslog, etc. If we really want to insist that std.logger provides selective control of libraries, then I think we will also need to establish policy at this time. I see two reasonable choices. Option 1: minimal change 1. The policy is that we control library logging by letting the client replace Logger references at runtime. 2. We say that libraries either use stdlog, i.e. info(...), error(...), etc., OR that they expose a Logger reference that clients can replace. 3. Clients replace stdlog or the libraries' Loggers with Loggers from their chosen framework. Option 2: invasive change 1. We make policy that there is a namespace of all loggers and it is __MODULE__. 2. stdlog is removed. 3. Loggers are obtained only via a getLogger() type call. Any function that wants to log must call getLogger() first. 4. log(...), info(...), etc. are removed. All logging goes through a Logger: Logger.info/error/etc(...) 5. Clients replace the getLogger() factory method with a function/delegate provided by their framework. I think that option 1 would scale better long-term. Phobos can choose to use stdlog for its own logging, and other libraries can make their own (vibelog, foolog, ...). std.logger would need no changes. Final thought: filtering. We have several filtering methods now, and I am confused as to what is actually missing: * ANY AND ALL logging can be disabled at compile time via versions. This only includes client code and libraries the client is compiling too; I would assume that the default shipped phobos would not version-remove log calls. * ALL logging can be disabled at runtime via globalLogLevel and a single if check at the call site. * Per-Logger logging can be disabled at runtime via Logger.logLevel and a single if check at the call site. * All other kinds of filtering (on msg, timestamp, method, etc.) are available at runtime to Logger subclasses, but happens after the logImpl call and setup of LogEntry. What other capabilities are needed for filtering?
Oct 28 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 26 October 2014 at 22:12:20 UTC, Martin Nowak wrote:
 On 10/25/2014 06:43 PM, Dicebot wrote:
 Because of that I am going to start voting despite some 
 arguments being
 still in process. I hope that won't cause any tension.
The dependency on external version identifiers in phobos is still a complete bummer and there are still many implementation issues. One reason why this is taking so long is that there were many issues some of which still need to be addressed.
I don't consider it a major issue as I don't think std.logger should be used inside Phobos at all.
Oct 27 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 27 October 2014 at 07:03:11 UTC, Dicebot wrote:
 I don't consider it a major issue as I don't think std.logger 
 should be used inside Phobos at all.
Yes, using std.logger inside of phobos is a no-no
Oct 27 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 27 October 2014 at 08:23:48 UTC, Robert burner Schadek 
wrote:
 On Monday, 27 October 2014 at 07:03:11 UTC, Dicebot wrote:
 I don't consider it a major issue as I don't think std.logger 
 should be used inside Phobos at all.
Yes, using std.logger inside of phobos is a no-no
Ayway, let's come with an agreement/compromise with Martin and I'll start voting immediately after.
Oct 27 2014
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 27 October 2014 at 12:03:33 UTC, Dicebot wrote:
 Ayway, let's come with an agreement/compromise with Martin and 
 I'll start voting immediately after.
Well, as far as I can see his argument was based on old code that has long been rewritten and he hasn't answered since I pointed that out.
Oct 27 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/27/2014 01:36 PM, Robert burner Schadek wrote:
 Well, as far as I can see his argument was based on old code that has
 long been rewritten and he hasn't answered since I pointed that out.
How do come to that insight?
Oct 27 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Monday, 27 October 2014 at 22:20:04 UTC, Martin Nowak wrote:
 On 10/27/2014 01:36 PM, Robert burner Schadek wrote:
 Well, as far as I can see his argument was based on old code 
 that has
 long been rewritten and he hasn't answered since I pointed 
 that out.
How do come to that insight?
because the code you show as an example: "cat > lib.d << CODE version (StdLoggerDisableLogging) enum isLoggingActive() = false; else enum isLoggingActive() = true; void doSome() { import std.stdio; writeln("loggingLib: ", isLoggingActive!()); } CODE" is different from the code that has been in the PR for quite some time. And the code you show does exactly what you say and the current code does something different.
Oct 27 2014
parent reply Martin Nowak <code+news.digitalmars dawg.eu> writes:
On 10/28/2014 01:01 AM, Robert burner Schadek wrote:
 is different from the code that has been in the PR for quite some time.
 And the code you show does exactly what you say and the current code
 does something different.
No it behaves the same. isLoggingActive is a template in phobos doSome is a function in a lib that performs logging and instantiates isLoggingActive main is a function in the app that performs logging and instantiates isLoggingActive and also calls doSome Now which of those functions actually logs depends on the compilation settings of the library, the compilation settings of the app and the logger that's being used.
Oct 27 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 28 October 2014 at 01:42:12 UTC, Martin Nowak wrote:
 On 10/28/2014 01:01 AM, Robert burner Schadek wrote:
 is different from the code that has been in the PR for quite 
 some time.
 And the code you show does exactly what you say and the 
 current code
 does something different.
No it behaves the same. isLoggingActive is a template in phobos doSome is a function in a lib that performs logging and instantiates isLoggingActive main is a function in the app that performs logging and instantiates isLoggingActive and also calls doSome Now which of those functions actually logs depends on the compilation settings of the library, the compilation settings of the app and the logger that's being used.
The second two are wanted and disabling a LogLevel at CT of phobos should be banned anyway. But no the less, it is one more option the user has to manipulate the Logger.
Oct 28 2014
parent "Martin Nowak" <code dawg.eu> writes:
On Tuesday, 28 October 2014 at 09:00:54 UTC, Robert burner 
Schadek wrote:
 The second two are wanted and disabling a LogLevel at CT of 
 phobos should be banned anyway. But no the less, it is one more 
 option the user has to manipulate the Logger.
And this is where the leakage happens because there is no sharp border between app and library. Functions of the library may get unlined into the app, templates of the library are instantiated in the app and some instantiations that are already in the lib won't be reinstantiated in the app.
Oct 28 2014
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 10/27/14 5:03 AM, Dicebot wrote:
 On Monday, 27 October 2014 at 08:23:48 UTC, Robert burner Schadek wrote:
 On Monday, 27 October 2014 at 07:03:11 UTC, Dicebot wrote:
 I don't consider it a major issue as I don't think std.logger should
 be used inside Phobos at all.
Yes, using std.logger inside of phobos is a no-no
Ayway, let's come with an agreement/compromise with Martin and I'll start voting immediately after.
Agreed. Just to restate my position: so long as we don't have a way to statically control maximum logging level in the client, we don't have a logging library. There is no negotiation. -- Andrei
Oct 28 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Tuesday, 28 October 2014 at 16:05:19 UTC, Andrei Alexandrescu 
wrote:
 Agreed. Just to restate my position: so long as we don't have a 
 way to statically control maximum logging level in the client, 
 we don't have a logging library. There is no negotiation. -- 
 Andrei
We have way to statically control logging level of the client from the client. Argument is mostly about precompiled 3d party libraries.
Oct 28 2014
parent Johannes Pfau <nospam example.com> writes:
Am Tue, 28 Oct 2014 16:32:34 +0000
schrieb "Dicebot" <public dicebot.lv>:

 On Tuesday, 28 October 2014 at 16:05:19 UTC, Andrei Alexandrescu 
 wrote:
 Agreed. Just to restate my position: so long as we don't have a 
 way to statically control maximum logging level in the client, 
 we don't have a logging library. There is no negotiation. -- 
 Andrei
We have way to statically control logging level of the client from the client. Argument is mostly about precompiled 3d party libraries.
Could somebody summarize that discussion? You can't 'statically' disable logging in a _precompiled_ library. So is this about statically disabling logging in libraries when source code is available or disabling logging at runtime for precompiled libraries?
Oct 29 2014
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 10/25/14 9:43 AM, Dicebot wrote:
 Jut for the reference, my position on current state of things as review
 manager is this:

 I agree with some of mentioned concerns and would like to see those
 fixed. However, I don't think any of those are truly critical and this
 proposal has been hanging there for just too long. In my opinion it will
 be more efficient to resolve any remainining issues in follow-up pull
 requests on case by case basis then forcing Robert to do it himself -
 there will be some time left before next release to break a thing or two
 before public statement is made.

 Because of that I am going to start voting despite some arguments being
 still in process. I hope that won't cause any tension.
Being able to select maximum logging level statically at client application level is a deal maker/breaker for me. The mechanics aren't important but it's likely they will affect the API. So I think that needs to be resolved now, not in a future pull request. Andrei
Oct 27 2014
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 28 October 2014 at 05:44:48 UTC, Andrei Alexandrescu 
wrote:
 Being able to select maximum logging level statically at client 
 application level is a deal maker/breaker for me. The mechanics 
 aren't important but it's likely they will affect the API. So I 
 think that needs to be resolved now, not in a future pull 
 request.

 Andrei
please elaborate. It is a must to be able to disable LogLevel when calling the compiler? Xor the opposite? passing -version=StdLoggerDisableTrace when compiling the user code will yield empty functions (thank you static if) for every call to any function or method having the word "trace" in its name, like for example trace("My log call"); If you call something like log(computeMyLogLevelAtRuntime(), "my log data) and that function returns LogLevel.trace and you haved passed -version=StdLoggerDisableTrace at CT of the user code you will have to pay for one if.
Oct 28 2014
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 10/28/14 2:46 AM, Robert burner Schadek wrote:
 On Tuesday, 28 October 2014 at 05:44:48 UTC, Andrei Alexandrescu wrote:
 Being able to select maximum logging level statically at client
 application level is a deal maker/breaker for me. The mechanics aren't
 important but it's likely they will affect the API. So I think that
 needs to be resolved now, not in a future pull request.

 Andrei
please elaborate. It is a must to be able to disable LogLevel when calling the compiler? Xor the opposite? passing -version=StdLoggerDisableTrace when compiling the user code will yield empty functions (thank you static if) for every call to any function or method having the word "trace" in its name, like for example trace("My log call"); If you call something like log(computeMyLogLevelAtRuntime(), "my log data) and that function returns LogLevel.trace and you haved passed -version=StdLoggerDisableTrace at CT of the user code you will have to pay for one if.
isFloatingPoint is an exact check, which is fine. We'd need isLikeXxx templates that support Liskov. -- Andrei
Oct 28 2014
prev sibling parent "Kevin Lamonte" <kevindotlamnodotonte gmail.com> writes:
On Saturday, 25 October 2014 at 16:43:10 UTC, Dicebot wrote:
 However, I don't think any of those are truly critical and this 
 proposal has been hanging there for just too long. In my 
 opinion it will be more efficient to resolve any remainining 
 issues in follow-up pull requests on case by case basis then 
 forcing Robert to do it himself - there will be some time left 
 before next release to break a thing or two before public 
 statement is made.
(Am I a voter? No idea. But here is my feedback, based on my recent experience implementing a fuller backend for std.logger.) I agree, I think std.logger is ready to go forward and needs to get into end-user hands. I see four areas that are incomplete but SHOULD be deferred to future bug reports / enhancement requests: 1. Additional LogEntry fields: Thread, Fiber, Throwable, maybe others. These should only be adopted if the end users can provide a compelling case for why their Logger subclasses (or backend framework) can't give them what they want. 2. Additional Logger functions: caught(), throwing(), maybe others. These should be driven by the needs of phobos' use of std.logger. (And phobos SHOULD use std.logger and eat its own dog food.) 3. Modification of the order or method in which the Logger functions obtain their data in constructing the LogEntry, i.e. call Clock.currTime() differently or not at all. These should be controlled by future additional fields/flags in Logger, with no changes to the functions one calls on a Logger. 4. Control of logging across multiple libraries/applications. This should absolutely be deferred until phobos itself starts using std.logger. Of these issues, I think that #4 is a huge can of worms that is impossible to make everyone happy, and we keep poking into it as a result of not accepting that the only people who can decide on it are the application writers, not the language and phobos designers. The two solutions for it at the extremes are: A) Deciding if a Logger is enabled when it is obtained by the client (via a Logger.getLogger() factory function), which requires imposing a namespace of some kind that ALL future libraries will have to adhere to. Note that log4j -- currently the fastest and "biggest" logging system out there -- started with a fixed className namespace, but moved away from that later on to more general user-defined "categories". Even in a straightjacketed language like Java one namespace isn't good enough. B) Deciding if a Logger should emit (based on end-user decision via config file, environment variable, ...) at the Logger.writeLogEntry() call via filtering on the LogEntry fields. Option A is cumbersome in client code but very fast in practice because getLogger() can set the Logger's logLevel to filter messages before the logImpl() call. Option B is easier in client code (and also lets many functions/modules/libraries write to a single Logger ala stdlog) but very slow at runtime because the namespace filtering can't occur until after the LogEntry is fully formed (formatted (including allocation) + Clock.currTime + ...). The application writers are the only people in a position to choose which end of this tradeoff makes the most sense for them. Phobos will have to choose for itself someday, and I think that various subsystems may very well choose differently based on their most common usage. std.net.curl might choose option B (just logging to stdlog) because it's got this nice clearly-defined border between the client and libcurl; whereas std.range might go for option A (letting the users set a getLogger() delegate) because it is very cross-cutting and users will only want to see log entries when doing range stuff in their code and not all over phobos or other libraries.
Oct 29 2014
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
Anyone know anything about this?

https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
Nov 24 2014
next sibling parent reply "Brian Schott" <briancschott gmail.com> writes:
On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You are posting to page 16 of the third iteration of a single review.
Nov 24 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/24/2014 4:51 PM, Brian Schott wrote:
 On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You are posting to page 16 of the third iteration of a single review.
I know, and the reddit comment refers to this.
Nov 24 2014
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Tuesday, 25 November 2014 at 01:12:03 UTC, Walter Bright wrote:
 On 11/24/2014 4:51 PM, Brian Schott wrote:
 On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright 
 wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You are posting to page 16 of the third iteration of a single review.
I know, and the reddit comment refers to this.
This discussion is indeed most unsettling to read. Third review of a much-needed module in the ecosystem, and I remember of previous attempts at logging, each time taken down because it does not satisfy the whims of top-tier D developers that would have done it differently (and of course "better"). What is accepted or not in Phobos no longer interest me. I can rely on interesting modules through DUB which has versionned dependencies, while Phobos has not. Better XML parsers/JSON parsers/serialization/argument parsers exist outside of Phobos currently, and in my opinion maybe they didn't belong there in the first place.
Nov 25 2014
next sibling parent "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Tuesday, 25 November 2014 at 14:29:12 UTC, ponce wrote:
 Better XML parsers/JSON parsers/serialization/argument parsers 
 exist outside of Phobos currently, and in my opinion maybe they 
 didn't belong there in the first place.
Yes, it is often a good idea to standardize after solutions have been established. Creating a good logger is harder than it sounds like as the logger have to used by everyone or else you have to deal with N incompatible versions in the same project due to different libraries using different loggers. Starting with the interface with no performant proof of concept means you go for a long run. Starting with an existing performant solution and abstracting it into a cleaner interface would reach completion faster and with less risk of hitting gotchas later. A logger that isn't good enough for just about everyone will become dead weight as people will gravitate towards an external adhoc standard solution instead (re Tango vs Phobos).
Nov 25 2014
prev sibling parent "Martin Nowak" <code dawg.eu> writes:
On Tuesday, 25 November 2014 at 14:29:12 UTC, ponce wrote:
 On Tuesday, 25 November 2014 at 01:12:03 UTC, Walter Bright 
 wrote:
 On 11/24/2014 4:51 PM, Brian Schott wrote:
 On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright 
 wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You are posting to page 16 of the third iteration of a single review.
I know, and the reddit comment refers to this.
This discussion is indeed most unsettling to read. Third review of a much-needed module in the ecosystem, and I remember of previous attempts at logging, each time taken down because it does not satisfy the whims of top-tier D developers that would have done it differently (and of course "better").
Things in phobos just have to sit, we already carry around too many crap modules (signals, XML, curl).
 What is accepted or not in Phobos no longer interest me. I can 
 rely on interesting modules through DUB which has versionned 
 dependencies, while Phobos has not.
That's a good thing because a package system can cover different needs with much more variety.
 Better XML parsers/JSON parsers/serialization/argument parsers 
 exist outside of Phobos currently, and in my opinion maybe they 
 didn't belong there in the first place.
I partly agree with this, having certain things covered by 3-rd party libraries allows for faster iteration. Something shpuld only become a Phobos module if there can be definite design which is paired with a very good implementation.
Nov 29 2014
prev sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You mean the second part, about him leaving D because of the discussion about the logger?
Nov 25 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/25/2014 2:26 AM, Robert burner Schadek wrote:
 On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You mean the second part, about him leaving D because of the discussion about the logger?
Yes.
Nov 25 2014
next sibling parent "Robert burner Schadek" <rburners gmail.com> writes:
On Tuesday, 25 November 2014 at 23:41:51 UTC, Walter Bright wrote:
 On 11/25/2014 2:26 AM, Robert burner Schadek wrote:
 On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright 
 wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You mean the second part, about him leaving D because of the discussion about the logger?
Yes.
Not really, this is the first time I read the name SiCl4. Also google SiCl4 dlang only points to the reddit post.
Nov 26 2014
prev sibling parent "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Tuesday, 25 November 2014 at 23:41:51 UTC, Walter Bright wrote:
 On 11/25/2014 2:26 AM, Robert burner Schadek wrote:
 On Tuesday, 25 November 2014 at 00:37:00 UTC, Walter Bright 
 wrote:
 Anyone know anything about this?

 https://www.reddit.com/r/programming/comments/2n9gfb/d_is_for_data_science/cmbssac
You mean the second part, about him leaving D because of the discussion about the logger?
Yes.
He wants a "better C" for system level programming: https://www.reddit.com/r/programming/comments/2n60wv/the_first_enemy_of_c_is_its_past/cmbn1n4 https://www.reddit.com/r/programming/comments/2n60wv/the_first_enemy_of_c_is_its_past/cmb4ows And there is no doubt an open spot between C and C++ for a modernized system level language.
Nov 26 2014
prev sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
recent updates:
* Martins CT log function disabling (thanks Martin)
* new thread local indirection Logger between free standing log 
functions and program global Logger
* more documentation
* some  trusted have been remove (thanks Dicebot)
* local imports

please review
Jan 06 2015
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/6/15 8:51 AM, Robert burner Schadek wrote:
 recent updates:
 * Martins CT log function disabling (thanks Martin)
 * new thread local indirection Logger between free standing log
 functions and program global Logger
 * more documentation
 * some  trusted have been remove (thanks Dicebot)
 * local imports

 please review
Links for the lazy. Code: https://github.com/D-Programming-Language/phobos/pull/1500 Dox: http://burner.github.io/phobos/phobos-prerelease/std_experimental_logger_core.html http://burner.github.io/phobos/phobos-prerelease/std_experimental_logger_filelogger.html http://burner.github.io/phobos/phobos-prerelease/std_experimental_logger_multilogger.html http://burner.github.io/phobos/phobos-prerelease/std_experimental_logger_nulllogger.html Andrei
Jan 06 2015
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/6/15 8:51 AM, Robert burner Schadek wrote:
 recent updates:
 * Martins CT log function disabling (thanks Martin)
 * new thread local indirection Logger between free standing log
 functions and program global Logger
 * more documentation
 * some  trusted have been remove (thanks Dicebot)
 * local imports

 please review
I propose we pull this in today and make it available for 2.067 as std.experimental.logger. We've been through a number of iterations with this and the best way to move forward is to accumulate a bit of real-world experience with it. Since we're deploying to std.experimental there is understanding breaking changes are still possible. Dicebot, as the review manager you get to decide. What do you say? Andrei
Jan 23 2015
next sibling parent "Joseph Cassman" <jc7919 outlook.com> writes:
On Friday, 23 January 2015 at 20:24:54 UTC, Andrei Alexandrescu 
wrote:
 On 1/6/15 8:51 AM, Robert burner Schadek wrote:
 recent updates:
 * Martins CT log function disabling (thanks Martin)
 * new thread local indirection Logger between free standing log
 functions and program global Logger
 * more documentation
 * some  trusted have been remove (thanks Dicebot)
 * local imports

 please review
I propose we pull this in today and make it available for 2.067 as std.experimental.logger. We've been through a number of iterations with this and the best way to move forward is to accumulate a bit of real-world experience with it. Since we're deploying to std.experimental there is understanding breaking changes are still possible. Dicebot, as the review manager you get to decide. What do you say? Andrei
+1
Jan 23 2015
prev sibling next sibling parent "Kapps" <opantm2+spam gmail.com> writes:
On Friday, 23 January 2015 at 20:24:54 UTC, Andrei Alexandrescu 
wrote:
 I propose we pull this in today and make it available for 2.067 
 as std.experimental.logger.

 We've been through a number of iterations with this and the 
 best way to move forward is to accumulate a bit of real-world 
 experience with it. Since we're deploying to std.experimental 
 there is understanding breaking changes are still possible.

 Dicebot, as the review manager you get to decide. What do you 
 say?


 Andrei
Agreed. Logger is such a subjective module. No matter what you do, there is a guarantee that people will have something more they desire. D badly needs at least some basic logging though, and if people need more complex features, they then do their own. But so long as the default is fine for most people, which this looks like it should be, then having it is a valuable addition; it's an important module. At this point, the most useful next step would be experience as you mention. There might be some improvements desired before full Phobos integration, such as cleaning up the documentation for some symbols (particularly the 12 'alias trace/info/etc(f)') by using version(D_Doc) to hide part of it, but that's certainly not necessary for std.experimental inclusion.
Jan 23 2015
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Friday, 23 January 2015 at 20:24:54 UTC, Andrei Alexandrescu 
wrote:
 I propose we pull this in today and make it available for 2.067 
 as std.experimental.logger.

 We've been through a number of iterations with this and the 
 best way to move forward is to accumulate a bit of real-world 
 experience with it. Since we're deploying to std.experimental 
 there is understanding breaking changes are still possible.

 Dicebot, as the review manager you get to decide. What do you 
 say?


 Andrei
I was in favor of merging it long time ago. It is _your_ defintion of being good enough for std.experimental I am trying to comply now ;) Merging it right now and continuing with fixes as follow-up pull requests is a good idea but you must understand that chance of breaking chance is very high. That said I have just now tested last version (with thread-local log support) and apart from one bug found it works as expected. Assuming you give your LGTM I will merge the PR as soon as this bug is fixed and PR is rebased/squashed.
Jan 24 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/24/15 7:06 AM, Dicebot wrote:
 On Friday, 23 January 2015 at 20:24:54 UTC, Andrei Alexandrescu wrote:
 I propose we pull this in today and make it available for 2.067 as
 std.experimental.logger.

 We've been through a number of iterations with this and the best way
 to move forward is to accumulate a bit of real-world experience with
 it. Since we're deploying to std.experimental there is understanding
 breaking changes are still possible.

 Dicebot, as the review manager you get to decide. What do you say?


 Andrei
I was in favor of merging it long time ago. It is _your_ defintion of being good enough for std.experimental I am trying to comply now ;) Merging it right now and continuing with fixes as follow-up pull requests is a good idea but you must understand that chance of breaking chance is very high. That said I have just now tested last version (with thread-local log support) and apart from one bug found it works as expected. Assuming you give your LGTM I will merge the PR as soon as this bug is fixed and PR is rebased/squashed.
Fantastic. Thanks! Robert, please mind the bug and then Dicebot please do the merge honors. Thanks Robert for this work, Dicebot for overseeing the review process, and everybody in the community who provided feedback and ideas. Andrei
Jan 24 2015
next sibling parent reply "Robert burner Schadek" <rburners gmail.com> writes:
I will fix the bug this weekend and rebase to upstream/master. 
The alias doc has seen some updates this week (please check the 
rebuild gh-pages, links are in the PR description)
Jan 24 2015
parent reply Jacob Carlborg <doob me.com> writes:
On 2015-01-24 18:45, Robert burner Schadek wrote:
 I will fix the bug this weekend and rebase to upstream/master. The alias
 doc has seen some updates this week (please check the rebuild gh-pages,
 links are in the PR description)
Is the generated documentation for package module available somewhere? This [1] is empty. [1] http://burner.github.io/phobos/phobos-prerelease/std_experimental_logger_package.html -- /Jacob Carlborg
Jan 25 2015
parent reply "Robert burner Schadek" <rburners gmail.com> writes:
On Sunday, 25 January 2015 at 09:43:36 UTC, Jacob Carlborg wrote:
 On 2015-01-24 18:45, Robert burner Schadek wrote:
 I will fix the bug this weekend and rebase to upstream/master. 
 The alias
 doc has seen some updates this week (please check the rebuild 
 gh-pages,
 links are in the PR description)
Is the generated documentation for package module available somewhere? This [1] is empty. [1] http://burner.github.io/phobos/phobos-prerelease/std_experimental_logger_package.html
fixed
Jan 25 2015
parent reply zeljkog <zeljkog home.com> writes:
On 25.01.15 15:06, Robert burner Schadek wrote:
 On Sunday, 25 January 2015 at 09:43:36 UTC, Jacob Carlborg wrote:
 Is the generated documentation for package module available somewhere? 
 This [1] is empty.

 [1] 
 http://burner.github.io/phobos/phobos-prerelease/std_experimental
logger_package.html 
fixed
Interface looks to me convenient. Is there a way to log to buffer or a like?
Jan 25 2015
parent "Dicebot" <public dicebot.lv> writes:
On Sunday, 25 January 2015 at 14:50:46 UTC, zeljkog wrote:
 Is there a way to log to buffer or a like?
By implementing own Logger. Idea is that std.experimental.logger provides only basic building blocks and defines API - any more advanced logger implementations can be provided via dub packages.
Jan 25 2015
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
Sadly, the issue I have been referring to is actually a DMD bug : 
https://issues.dlang.org/show_bug.cgi?id=14050

It seems a blocker, at least I can't quickly imagine a workaround 
for it.
Jan 26 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/26/15 6:52 AM, Dicebot wrote:
 Sadly, the issue I have been referring to is actually a DMD bug :
 https://issues.dlang.org/show_bug.cgi?id=14050

 It seems a blocker, at least I can't quickly imagine a workaround for it.
Thanks Kenji for the fix! Just merged it. -- Andrei
Jan 26 2015
prev sibling parent reply Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Tuesday, 6 January 2015 at 16:51:26 UTC, Robert burner Schadek 
wrote:
 please review
Two small doc issues on http://dlang.org/phobos/std_experimental_logger.html "The LogLevel of an log call" -> "[...] a log call" "[...] or The additional f enables printf-style logging" missing parts of a preceding sentence? Bastiaan.
Jan 06
parent Bastiaan Veelo <Bastiaan Veelo.net> writes:
On Wednesday, 6 January 2016 at 16:30:23 UTC, Bastiaan Veelo 
wrote:
 Two small doc issues on 
 http://dlang.org/phobos/std_experimental_logger.html
https://github.com/D-Programming-Language/phobos/pull/3907
Jan 07