[Feedback] Server Logging API (with revisions)


(Ravi Kandhadai Madhavan) #61

@johannesweiss Sorry this is coming later in the review. We’ve been working on trying to align the usage between os_log and this proposal. We have some suggestions in this regard, which, besides os_log, may also be useful in supporting more backends.

Pinning down the meaning of Log Levels

This will make it easier and less ambiguous to map the log levels to those offered by an existing logging system like os_log. For instance, syslog, which has thorough definitions for these levels and is also an industry standard, could be one good reference point to relate the log levels with.

Adding a log-level above Error (that may perform expensive operations)

os_log has a log level above error called fault. Many other systems like Syslog, log4j etc. also have one or more levels above error. Could we add this level too? This level would mean that the logging backend could choose to capture much more information, such as a stackshot, to aid with debugging a major issue.

Allowing the log level to be omitted

Can the log-level parameter in the log method be optional? When the level is not specified the backend can choose a default level and log to it. The experience with os_log seems to be that it is better to not force users to choose a log level.

Enabling log creation using initializers

As mentioned in other comments, to be consistent with the Swift naming guideline, can we use initializers rather than static methods for creating loggers, as illustrated below? Is there any disadvantages of doing that?

let  logger = Logger("myapp")

The initializer of Logger can query the Logging._factory and initialize the log handler property as sketched below:

struct Logger {
  public init(label: String) {
     self.handler = Logging._factory(label)
  }
}

Making the logger instances immutable

It seems metadata and minimum enabled log-level are mutable properties of the Logger struct. Would it be better to make them return new instance of the logger leaving the earlier instance unchanged? This may avoid problems such as forgetting to reset the metadata on a shared instance etc.

cc @Ben_Cohen, @Devin_Coughlin, @moiseev


(Johannes Weiss) #62

Hi Ravi,

Thanks for your feedback!

:+1:

Absolutely, let's add fatal or fault.

What experience with os_log was the deciding factor to allow level-less logging? Did os_log ever force users to provide a logging level and the were opposed to doing that? I really can't see the benefit of that because the only really way out would be to say that 'no level' == info or something, right?

I would prefer not to intertwine Logger and Logging the global logger factory because I think it's better if Logging does that nasty globals stuff and Logger is the type users deal with when logging.
There's also precent in Swift for factories for example makeIterator(). But if that's the only real issue we have let's discuss this more.

Correct, they are mutable. The preferred way is that Loggers act like a value-typed logging configuration. Sure, the actual logging backend will almost always be a reference (for example a reference to an open file) but those parts are immutable. When I say logging configuration I mean with that all the parts of a LogHandler that are mutable (mostly log level and metadata).

Let's make this more concrete: This is an example

func spawnNewClient(logger serverLogger: Logger) {
    var clientLogger = serverLogger
    let clientUUID = UUID()
    clientLogger[metadataKey: "request-uuid"] = "\(clientUUID)"
    clientLogger[metadataKey: "request-start-time"] = "\(Date())"
    clientLogger.logLevel = logLevelForRequest(logger: clientLogger)
    runClient(logger: clientLogger)
    serverLogger.info("spawned new client: \(clientUUID)")
}

The logLevelForRequest function would for example usually return clientLogger.logLevel but say every 10,000th request it would return .debug. That's very useful if you want to sample every 10,000th request in .debug log-level and most others in your usual log level like .warn or .info.

Now because the preferred way is for the Loggers to have a value-typed logging configuration (in a struct MyLogHandler: LogHandler { ... } the effect of this is that serverLogger is unchanged, ie. we only ever change the logging level for one particular clientLogger. So we're just using Swift's usual mutation syntax to mutate the Logger instead of creating a new logger and modifying it on creation.

Of course, we could've done what languages without value semantics have to do:

// THIS IS NOT THE PROPOSAL, JUST DEMONSTRATING ALTERNATIVE SYNTAX
func spawnNewClient(logger serverLogger: Logger) {
    let clientUUID = UUID()
    let clientLogger = serverLogger.addMetadata("request-uuid", "\(clientUUID)")
                                   .addMetadata["request-start-time", "\(Date())")
                                   .logLevel(logLevelForRequest(logger: clientLogger))
    runClient(logger: clientLogger)
    serverLogger.info("spawned new client: \(clientUUID)")
}
// We need to be somewhat careful here because we'll throw two intermediate `Logger`s away until
// we get to build the final logger. Should be fine though assuming `Logger` is a `struct`.

So now you could call Logger immutable. Swift however allows us to use value semantics which combines the nice syntax of mutability and all the benefits of immutability. And most crucially, the end result is the same: serverLogger is unchanged even though clientLogger (which is derived from serverLogger) is changed.

Here, I do need to point out that having a value-typed (for the configuration) LogHandler is only the preferred way to do things. There will always be people who want to change the log level of all existing loggers globally and whilst I don't think that's a great idea the proposal allows this by making reference-typed LogHandlers. In the previous responses I alluded to this as 'squaring the circle' and that's what it is: The intended use is for the logging configuration to be held as a value (in a struct MyLogHandler: LogHandler) but if some application really wants one global logger, then they could use a reference-typed LogHandler (with all the caveats that apply, like metadata will be attached globally...).


(Xiaodi Wu) #63

Particularly if this design is to be emulated by system logging libraries, I would echo Ravi that it's important to observe Swift idioms as much as possible.

  • I would reiterate that the naming of LogHandler, Logging, and Logger are unusual and need clarification. The explanations in the prose are quite clear, but they are nowhere reflected in the names. All three types having something to do with "logging" (and the type that actually does the logging isn't named Logging!), which naturally mean they "handle logs." Perhaps LogImplementation (brevity is not necessary here), LogFactory, Log.

  • Factories are pretty rare in Swift. Unless something really can't be expressed as an initializer, I would agree that every effort ought to be made to use them.

  • In idiomatic Swift, configuring a type X typically occurs with methods on type X (usually static methods); I would recommend that the methods used to configure a Logger be moved from Logging to Logger, and the methods used to configure a LogHandler be moved from Logging to LogHandler, thus condensing three entities to two.

  • In Logger, info(...), etc., are duplicative of log(.info, ...) and not in line with naming guidelines (since nouns are used for nonmutating methods, and logging is an I/O operation, which in Swift is expressed with verbs); I would recommend their removal.

  • In Logger and LogHandler, as mentioned by others, subscript(metadataKey:) and metadata provide duplicative interfaces; both should not be present as top-level API even if there's some thread-local functionality enabled by one but not the other. If necessary, one can be sunk deeper, but the API design should be obvious in terms of reflecting what's the preferred usage. To me it seems that since .metadata[key] allows that thread-local functionality mentioned earlier and is more consistent with Swift naming guidelines (we can omit writing "key" in a subscript label, for one), it would seem to be the superior choice.


(Ravi Kandhadai Madhavan) #64

Just wanted to pitch in a thought here. It may not be necessary to create two intermediate instances. There can be one initializer (or some other method) of Logger that takes an existing logger (in this case serverLogger) and takes the new metadata and log-level as additional parameters and initializes/returns a new logger instance. E.g.

let clientLogger = Logger(other: serverLogger, metadata: [["request-uuid", "\(clientUUID)"],  "request-start-time", "\(Date())"]], minimumLogLevel: logLevelForRequest(logger: ...)) 

(Sorry, I may not be getting all the syntax right here for metadata etc.) From a compiler perspective, lets conveys more information to the compiler. We can get better guarantees and diagnostics with immutable values, it can reduce chances of bugs, and can have more scope for optimizations. Those are some benefits which can be weighed against the loss in flexibility due to lets and how contrived the APIs become. Just a limited thought I had.


(Johannes Weiss) #65

Sure, you can build a DSL like you propose, we also need at least removal of metadata so maybe

Logger.init(other: Logger,
            additionalMetadata: [String: Metadata.Key],
            removeMetadataKeys: [String])

And yes, you could also do metadata: [String: Metadata.Key?] where nil means 'remove'. Built I don't quite see the point in creating a DSL when Swift provides perfectly fine syntax.

I'm pretty sure there's a misunderstanding here: Your arguments are exactly right with reference semantics. I was however talking about types with value semantics and there your arguments don't apply.

I'm sure you know all of the above but I'll still going to write it down because I think others might also be confused by what we're proposing:

One of the best features I love so much about Swift is value semantics. In languages like ObjC, Python, Java, ... making everything immutable with 'mutation' methods that return a fresh instance make your program as you say safer for the reasons you outlined above. Shared mutable state is dangerous so we usually try to avoid it, having 'mutation' methods that return fresh instances remove the mutable in shared mutable state (because as you say they're immutable).

In Swift however we can create types with value semantics which is essentially eliminating shared in shared mutable state. The end result is the same: There is no shared mutable state which is the main objective. (Even in value types that are implemented with a reference backing (such as String, Array, Dictionary, Set, ...) we still eliminate shared mutable state by copying on write (if there's sharing)).

Can you give an examples here? With value types usually you get better optimisations because you're telling the compiler exactly what you want. Here's example code (this is not from the proposal, just to get across the point):

import Foundation

@inline(never)
func blackhole<T>(_ value: T) {
}

struct MetadataWrapperAsValue {
    var metadata: [String: String] = [:]

    subscript(metadataKey metadataKey: String) -> String? {
        get {
            return self.metadata[metadataKey]
        }
        set {
            self.metadata[metadataKey] = newValue
        }
    }
}

struct MetadataWrapperImmutable {
    let metadata: [String: String]

    init(metadata: [String: String]) {
        self.metadata = metadata
    }

    init() {
        self.init(metadata: [:])
    }

    init(other: MetadataWrapperImmutable, modifications: [String: String?]) {
        self = other.modify(modifications)
    }

    func modify(_ modifications: [String: String?]) -> MetadataWrapperImmutable {
        var newMetadata = self.metadata
        for mod in modifications {
            newMetadata[mod.0] = mod.1
        }
        return MetadataWrapperImmutable(metadata: newMetadata)
    }
}

let rounds = 1_000_000

@inline(never)
func testWithSwiftSyntax(_ m: MetadataWrapperAsValue) {
    for _ in 0..<rounds {
        var firstNew = m
        firstNew[metadataKey: "x"] = "y"

        var secondNew = firstNew
        secondNew[metadataKey: "x"] = nil

        blackhole(secondNew)
    }
}

@inline(never)
func testWithDSLSyntax(_ m: MetadataWrapperImmutable) {
    for _ in 0..<rounds {
        let firstNew = MetadataWrapperImmutable(other: m, modifications: ["x": "y"])
        let secondNew = MetadataWrapperImmutable(other: firstNew, modifications: ["x": nil])
        blackhole(secondNew)
    }
}

let start1 = Date()
testWithSwiftSyntax(MetadataWrapperAsValue())
let end1 = Date()

let start2 = Date()
testWithDSLSyntax(MetadataWrapperImmutable())
let end2 = Date()

for _ in 0..<5 {
    print("as value : \(end1.timeIntervalSince(start1))")
    print("immutable: \(end2.timeIntervalSince(start2))")
}

If we run this:

$ swift -O test.swift 
as value : 0.24369895458221436
immutable: 0.9814640283584595
as value : 0.24369895458221436
immutable: 0.9814640283584595
as value : 0.24369895458221436
immutable: 0.9814640283584595
as value : 0.24369895458221436
immutable: 0.9814640283584595
as value : 0.24369895458221436
immutable: 0.9814640283584595

which shows that the natural value type version (which has the same safety guarantees as the immutable one) is about 4x faster. Why is this? Because we need to do fewer copy-on-write copies here because by using the var we communicate to the compiler more clearly what we want to do.

Here's a dtrace command that'll show you all the allocations:

swiftc -O test.swift && sudo dtrace -n 'pid$target::malloc:entry { @mallocs[ustack()] = count(); } :::END { printa(@mallocs); } ' -c ./test

Long story short: The value version with natural mutation allocates once and the immutable version allocates 4 times. That's because in the value version with natural mutation we need trigger the copy-on-write copy exactly once (we need that) and in the immutable version we trigger it multiple times. We could probably optimise the code but the natural value version already does the optimal thing: On the first mutation it copies (has to because the caller holds that Dictionary too) but the second mutation is basically free because we're no longer using the firstNew.


(Ravi Kandhadai Madhavan) #66

First, thanks @johannesweiss for taking the time to write up the sample examples and measure their performance. I appreciate your efforts in trying to clarify these design choices.

I would also like make it clear that the fact that whether Logger type is a mutable value type or an immutable value type is not crucial for aligning the os_log and server-side APIs, at least in the initial stages. However, I would like to take this chance to clarify/explain on what I had in mind about using immutable Logger value type and why I thought it was better here.

Let us just focus on value types here. By immutable value types I mean structs that after initialization will not allow their (stored) properties to change. (The stored properties can themselves be private vars or lets.) It would not have mutating functions and non-private vars. If they support any updation at all, they must create and return new instance and cannot perform in place updates by definition. OTOH, mutable value types can perform in place updates through mutating functions or non-private mutable properties.

Now, I will use a little bit of functional programming terminology. Conceptually, there are two kinds of updates: ephemeral and persistent. An ephemeral update is when you update an instance and you are no longer interested in the earlier "state" of the instance. A persistence update is when you are interested in the earlier state and want to keep it alive.

One main reason to prefer an immutable type is when we want to allow only persistent updates. That is to impose a guarantee that whenever we update the earlier instance should remain unchanged. Immutable value types can enforce this independent of how they are used. (When mutable value types are used as lets (or passed as arguments) this guarantee gets enforced as well, but when they are used as var, in place updates are possible. So it is up to the client of the struct.)

Focusing on our particular logging scenario, the question is: whenever a Logger is updated do we want the previous logger to remain unchanged? It appeared to me that this was the guarantee we would like to impose for serverLogger and clientLogger example. We don't want a new spawnNewClient implementation to accidentally update the serverLogger's log level and metadata itself. (It cannot now as serverLogger is a parameter. But, could it be passed as an inout or can a global instance be shared in the future design?.) If we want the flexibility to destructively mutate Logger, then obviously immutability is not suitable here.

If we are interested in enforcing persistent updation of Logger and are only prevented by the performance of immutable implementation, then that seems addressable. The MetadataWrapperImmutable.init presented in @johannesweiss example seems a little inefficient.

For instance, consider the following initializer of MetadataWrapperImmutable and following persistent usage:

let instance1: MetadataWrapperImmutable =  [key1: value1, key2: value2 ... ]  
let instance2: MetadataWrapperImmutable =  MetadataWrapperImmutable(instance1, [key1 : newValue])

// Both instance1 and instance2 will be used later.

where the initializer is defined as follows:

struct MetadataWrapperImmutable { 
   public init(_ other: MetadataWrapperImmutable, _ modifications: [String: String]) { 
       self.metadata = other.metadata 
       for mod in modifications { 
          self.metadata.updateValue(mod.1, forKey: mod.0)  // Assuming this is an in place update and metadata is private mutable var
       }
   }

(There could be a more efficient way to in place update a set of keys in a dictionary directly instead of using updateValue.) It seems this might only create one more copy-on-write, namely for the passing of the parameter, compared to doing this with a mutating struct. In fact, with a more efficient implementation this can also be elided.

It seems to be me that the persistent update use case is representative of what happens with the clientLogger and serverLogger example presented earlier, where we copy the serverLogger into the clientLogger and mutate the clientLogger. We also need the serverLogger version (therefore the client copies it explicitly, which is an obligation on the client). Also, we never mutate the clientLogger after that initial mutation. The immutable strategy would do the same thing but hide the copy and mutation beneath an initializer and provide an immutable abstraction. So no matter how the client uses it, one cannot destroy the previous logger state.

I may be completely wrong in the assuming that the clientLogger is only persistently updated like in the above example. If the model is to allow mutating the same instance in place multiple times, making it immutable will not be the best thing to do.

P.S. I can share some of my thoughts on why immutable value types, when used in the right places, can be more performant than mutable value types by enabling more compiler optimization, if it may help in deciding on a design. But, as of now, I think the language usage considerations and guarantees that needs to be imposed would probably take higher precedence than whether some compiler optimization is possible or not.


(Johannes Weiss) #67

Most welcome, and thank you too @ravikandhadai, this is very important as I think we all want to reach a solution that is 1) as good as possible (given some requirements/constraints) and almost more importantly 2) everybody understands the tradeoffs.

Yes, I do too. But: Mutable value types and immutable value types that have functions returning new instances with a a property changes are the same thing. Therefore I tend to not talk of immutable values because the term is confusing (at least to me). Why confusing? Because any value type where we can obtain fresh instances with a given property altered is equivalent to a value type with that property mutated in place. Again, let's have an example. I'm using the example of a Point here but everything will apply to a value-typed Logger as well as any other value type in the same way.

struct PointImmutable {
    let x: Int
    let y: Int

    init(x: Int, y: Int) { self.x = x; self.y = y }

    func modifyX(_ newX: Int) -> PointImmutable {
        return PointImmutable(x: newX, y: self.y)
    }

    func modifyY(_ newY: Int) -> PointImmutable {
        return PointImmutable(x: self.x, y: newY)
    }
}

struct PointMutable {
    var x: Int
    var y: Int

    init(x: Int, y: Int) { self.x = x; self.y = y }
}

Semantically, PointMutable and PointImmutable are the same (even the in-memory representation will be 100% the same), the only difference is that PointMutable has better syntax and in certain cases can be optimised better. Both PointMutable and PointImmutable being value types means that on every assignment (doesn't matter if it's var new = old, let new = old, or calling a function with old) it will be copied. Again, let's have an example:

var globalVarMutable = PointMutable(x: 1, y: 2)
var globalVarImmutable = PointImmutable(x: 1, y: 2)
let globalLetMutable = PointMutable(x: 1, y: 2)
let globalLetImmutable = PointImmutable(x: 1, y: 2)

func increaseXby1() {
    globalVarMutable.x += 1
    globalVarImmutable = globalVarImmutable.modifyX(globalVarImmutable.x + 1)

    globalLetMutable.x += 1 // COMPILE ERROR
    globalLetImmutable = globalLetImmutable.modifyX(globalLetImmutable.x + 1) // COMPILE ERROR
}

// not actually possible because it's value types, so we can't modify the callers but this is to demonstrate the compiler catches it
func increaseParameters(pm: PointMutable, pi: PointImmutable) {
    pm.x += 1 // COMPILE ERROR
    pi = pi.modifyX(pi.x + 1) // COMPILE ERROR

    var pm2 = pm
    var pi2 = pi

    pm2.x += 1 // modification will _not_ affect `pm`
    pi2 = pi2.modifyX(pi2.x + 1) // modification will _not_ affect `pi`
}

func increaseParametersInout(pm: inout PointMutable, pi: inout PointImmutable) {
    pm.x += 1
    pi = pi.modifyX(pi.x + 1)
}

So they have the same capabilities, and behave the same. What's also possible is to transform one into the other, the usual way of proving equivalence:

struct PointMutableImplementedFromImmutable {
    private var backingStore: PointImmutable

    init(x: Int, y: Int) {
        self.backingStore = .init(x: x, y: y)
    }

    var x: Int {
        get {
            return self.backingStore.x
        }
        set {
            self.backingStore = self.backingStore.modifyX(newValue)
        }
    }

    var y: Int {
        get {
            return self.backingStore.y
        }
        set {
            self.backingStore = self.backingStore.modifyY(newValue)
        }
    }
}

struct PointImmutableImplementedFromMutable {
    private var backingStore: PointMutable

    init(x: Int, y: Int) {
        self.backingStore = .init(x: x, y: y)
    }

    func modifyX(_ newX: Int) -> PointImmutableImplementedFromMutable {
        var new = self // new and self are independent as we're a value type
        new.backingStore.x = newX
        return new
    }

    func modifyY(_ newY: Int) -> PointImmutableImplementedFromMutable {
        var new = self // new and self are independent as we're a value type
        new.backingStore.y = newY
        return new
    }
}

But for value types, every single time you assign it somewhere acts (or is) like a full copy of the whole value. So every update will only affect the very variable that you changed.

Yes, and that will be true for all value types. As I pointed out before: We recommend implementing value-typed LogHandlers and then this guarantee will be given as it is for all value types. If someone ignores the recommendation and implements a reference typed LogHandler then they won't get the desired semantics.

I think we must have another misunderstanding here. Would you mind providing an example where one can see the difference between a mutable and an immutable value type?
My reading of your last sentence is not accurate, var and let do exactly the same thing for both mutable and immutable value types. Whenever you pass or assign a value type's value to anything you have a fresh copy. For example

var myArray = [1, 2]

func foo(array: [Int]) {
    var array = array
    array.append(3)
}

foo(array: myArray)
print(myArray)

will print [1, 2]. The updating operation array.append(3) only affects the local variable array and not the global myArray even though it's var. And Array is a mutable value type. Yes, you could change the global if it were func foo(array: inout [Int]) and you were to call it as foo(array: &myArray). But the very same argument is true for 'immutable value types' as you can also assign a fresh value through an inout. See the increaseParametersInout example from above.

Thanks for that. I agree that my implementation is not optimal but the point stands: It's less efficient than the mutable version and the initial claim was that the compiler is often able to make it more efficient.

No, you're absolutely right. The recommendation is that LogHandlers are values, that makes Logger a value and this will work:

let logger1 = Logging.make("logger1")
var logger2 = logger1
logger2.logLevel = .error
var logger3 = logger2
logger3.logLevel = .warning
var logger4 = logger1
logger4.logLevel = .info

logger1.info("this might be logged, depending on the initial log level")

logger2.warning("this will not be logged as logger2's log level is .error")
logger3.info("this will not be logged as logger3's log level is .warning")
logger4.debug("this will not be logged as logger4's log level is .info")

So as for all value types, each variable is completely independent from all others. In exactly the same way as an "immutable value type" would work.


(Johannes Weiss) #68

Hi @xwu,

Thanks for your responses and sorry for the big delay.

Got it, I'm not overly concerned (but also not too happy about) LogHandler because it's for logging backend implementers only.
I understand your concern regarding Logging and Logger. We could go for LoggingSystem or something. I think however we're down to the details here and just like log levels I think we could discuss that in a separate thread. I'm happy to do that should this proposal go ahead in the SSWG.

Agreed. But this one I think is really quite important. We could make an initialiser that then takes the lock and reaches out to the global but that's really not very nice. I don't think/I hope that there's just not too many libraries out there that need an awful global like this. But the logging system needs to have one global backend (IMHO, see discussions with @anandabits).

I really think both interfaces have there place. logger.log(.info, "foo") just feels a bit noisy...

Agreed but we need to at least be able to wholesale swap the whole metadata (for MDC and the likes). But as @anandabits suggested, we could make this opaque (we already have an issue for that).


(Neal Lester) #69

One thing which is clear from the recent discussion is that understanding the implications of an immutable logger is more straightforward than understanding the implications of value semantics with a mutable logger. This isn't an important argument for using an immutable logger, but it does suggest that if the mutable design is selected the implications of its value semantics should be explicitly described in the documentation.

Upon further consideration, I'm not so sure about the above statement. Here is an example of a bug which would be much harder to write with the immutable logger design:

// SPECIFICATION: All log entries from Parent and Child shall include the log metadata ["parentId":Parent.id]

struct Parent {
    
    init (_ logger: Logger, id: Int) {
        self.logger = logger
        self.id = id
        self.child = Child (logger)
        self.logger[metadataKey: "parentId" = self.id]
    }
    
    var logger: Logger
    let id: Int
    let child: Child
    
}

struct Child {
    
    init (_ logger: Logger) {
        self.logger = logger
    }
    
    let logger: Logger
    
}

It does have the advantage of making dynamic log level selection simpler.


(Ravi Kandhadai Madhavan) #70

Would you call a value type (struct) with a mutating function on it (mutating func) or a public var an immutable type? I wouldn't call it so. For me, removing mutating funcs and public var properties on the Logger will essentially make it an immutable type. I would like to distinguish between a value type with a mutating func or public var property and a one without. A one without provides stronger guarantees, doesn't it? A client of the type cannot update an instance in place after creation. But the stronger guarantee also implies less flexibility. That is the tradeoff I would like to put forth.

With a PointMutable a client can do this:

var pmut = PointMutable(x: 1, y: 1)
var somecomp = {  print(pmut) } 
....
pmut.x = 10  // changes the sam
somecomp()  // will see the new value

whereas

var pmut = PointImmutable(x: 1, y: 1)
var somecomp = {  print(pmut) } 

// No matter what you do here, somecomp will always produce (1, 1)

That's the difference and guarantee I am referring to here. Btw, if you imagine somecomp is passed to a different thread and invoked from a different thread, and pmut is updated in the current thread, there will be races in the mutable design. AFAIK, value types with mutating funcs are not a guarantee that there wouldn't be any shared, mutable state or no races etc. It just makes it a bit difficult, as aliasing is much less common (unless you start you closures or inout's or globals). Immutable type guarantee no in place mutation.

cc @Ben_Cohen


(Johannes Weiss) #71

No. I'm saying for value types there's no difference between a type that's directly mutable (via mutable func or var) and one where you have only lets but modification functions which return you fresh instances with one property changed. I would call PointImmutable immutable and I would call PointMutable mutable and yet, they provide the same safety guarantees.

What I'm really saying is that for value types there is no meaningful difference whereas for reference types there's a massive difference and I wholeheartedly agree with everything you said, if it were about reference types.

That's not true, running that code

var pmut = PointImmutable(x: 1, y: 1)
var somecomp = {  print(pmut) }

pmut = PointImmutable(x: 200, y: 200)

somecomp()

will produce

PointImmutable(x: 200, y: 200)

What you wanted to write is

var somecomp = {  [pmut] in print(pmut) }

and again, it'll work the same for mutable and immutable. The [pmut] requests that the closure captures a copy of pmut. In my eye, it's a wrinkle in Swift that capturing a var really captures a reference to the storage of the var. But I guess that was made that the following code works fine:

let foo = [1, 2, 3]
var bar = []
foo.foreach {
    bar.append($0)
}

No, not if it's a value type.

There's two cases:

  1. the instance is stored in a let: neither the mutable nor the immutable can update in place
  2. the instance is stored in a var: the mutable one can update it, the immutable one can replace it with a fresh value. In both cases, the variable has been changed

That is an illegal program and it will be illegal for both mutable and immutable type. Remember, this is also illegal

struct ImmutableInt {
    let value: Int
}

var someInt = ImmutableInt(value: 5)
DispatchQueue(label: "x").async {
    print(someInt)
}
someInt = ImmutableInt(value: 7)

because the mutable thing here is the variable someInt not the type ImmutableInt.


(Ravi Kandhadai Madhavan) #72

Okay. I see that in that example you can replace it altogether and make the change visible. (So there can be shared, mutable state with vars loggers in any case.) But wouldn't you want to encourage the loggers to be always lets? Wouldn't the mutable design encourage creating var clientLogger and then mutating it, because there is no other way?


(Johannes Weiss) #73

Isn't that true for all of Swift's value types? Does that make Int, Float, Array, Dictionary, String, ... more error prone?


(Ravi Kandhadai Madhavan) #74

I can make i = j + 1 if I want, and can keep i and j lets. That is possible with the basic type. You can always just work with lets esp if there threads and sharing involved. But, it seems there no way I can work with just lets in the Logger even if I want to? Is there a way to change metadata non-destructively?


(Johannes Weiss) #75

Ok, this sounds like a good idea: Providing both, the mutating versions and non-destructive ones, totally open to that (like Array.sorting (non-destructive) vs. Array.sort (mutating)).

I don't consider this core because a user could add the non-destructive versions through an extension. But yeah, probably we should just add that to Logger so everybody can choose between destructive and non-destructive ones.


(Johannes Weiss) #76

I guess we can now for sure say that this is a good argument for an immutable API.
Irregardless of what languages a programmer is familiar with, reasoning about the immutable API is easy as it requires no distinction between value and reference types.

Said that, if we were to offer exclusivity an immutable API, we would lose the (unrecommended) choice of enforcing global log level across all loggers. In the proposed API that is possible by making LogHandler a reference type. Yes, that is against the recommendation and invalidates everything we were arguing about (as it makes Logger a reference type) but it allows the ‘there should only be one log level set for the whole program’ camp to use the API as well...


(Neal Lester) #77

Perhaps I'm not understanding the use case, but couldn't this be accomplished by giving the Logger:

let globalLogLevel: LogLevel?

which could be set by the factory. If present, derived loggers would use the global level instead of whatever was specified (if anything) with the derivation.


(Johannes Weiss) #78

@neallester yes, you could. However your program is likely composed of multiple libraries and those libraries might create their own loggers. Sure, it would be better if they accepted a logger passed in but we can’t force people into that design (API breaking & ‘too much hassle’). Therefore likely your program will be composed of multiple loggers obtained straight from the logging system.

The problem with logging is that it’s only really useful if everybody uses the same API (that includes 3rd party libraries). But people have such differing opinions about logging (compare for example @xwu’s comment in the first thread that there should be an always readily available global default logger vs. @anandabits opinion that it shouldn’t even be possible to create new root loggers and logger should be always handed explicitly to everywhere). Trying to accommodate at least existing solutions will lead to a tradeoff...


(Xiaodi Wu) #79

At multiple points, you have argued that certain design choices here are about enabling unrecommended patterns. Instead of accommodating incompatible opinions, I would say that the challenge of creating a good design is rationalizing why one is preferable to another and rejecting others.

What would a no-tradeoffs design look like? I think the community would be very interested in that.


(Neal Lester) #80

Loggers obtained from the logging system would be created by the factory which is under application developer control. If the application developer wanted to dispense all loggers at a specific log level and specify that level as their globalLogLevel (so that derived loggers would use the same level) the

"there should only be one log level set for the whole program"

use case could be accommodated with both the mutable and immutable logger designs. Thus, I don't understand how that use case is relevant to deciding between the mutable and immutable logger designs.

Another option: not allow derived loggers to change the log level.

I thought this question had been settled decisively in favor of providing a standard central logger factory.