std.experimental.logger formal review round 3

[Jakob Ovrum via Digitalmars-d]

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.