std.experimental.logger formal review round 3

[Robert burner Schadek via Digitalmars-d]

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