http://bugzilla.qos.ch/show_bug.cgi?id=100 joern@huxhorn.de changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED | ------- Comment #12 from joern@huxhorn.de 2008-03-13 04:00 ------- With "won't really solve the original problem" I meant that the array wouldn't serialize correctly in case of large Strings. And since the bug report says "Serialization of objectArray in LoggingEvent does not always work", and this was still true, it didn't really solve the problem. ;) I should've probably written "but introduces a different one instead" instead of "but introduces a whole new one instead". Sorry if this was confusing. However, I don't think that capping the parameters is acceptable. This is documented nowhere, neither in slf4j (where such a behavior would need to be documented) nor in Logback. I agree that it's probably arguable if log-messages/arguments of log-messages > 0xFFFF characters are reasonable or desirable but this should be up to the user and not the logging framework to decide. It sometimes *is* desirable to dump an xml structure that's larger than 64k and this really isn't absurd in case of log.debug or log.trace. That's why I reopened this bug. I'd also like to know what's wrong with my patch because I really do my best to adhere to your coding style, create minimal impact changes, add tests, etc. - It's less complex because it's using serialization instead of writing the array manually. - Changing the arguments to string is done only once and only if necessary. Serializing an event multiple times will not perform this operation again. - It works for Strings of any size because they are serialized instead of being written using writeUTF. - It creates the formattedMessage lazily if needed, therefore saving time during construction, increasing performance of the whole logging system. - Additionally, formattedMessage should be transient and would be recreated lazily from the message and the arguments after deserialization if needed. I didn't declare formattedMessage transient in my patch because I didn't want to change the serialized structure (which you just did anyway). Instead, I set formattedMessage to null in writeObject which has a comparable effect, i.e. it won't transfer duplicate data that can be derived from already available data (Message + Argument + MessageFormatter) instead. - it's completely transparent for classes using LoggingEvent itself because - the serialized data does not change it's form. - the lazy initialization of formattedMessage is invisible. It's simply not relevant, from the callers perspective, if formattedMessage is initialized in the c'tor or in the getFormattedMessage method. Your change, on the other hand - changes the serialized data without changing the serialVersionUID which produces strange errors while trying to deserialize old events. - creates capped strings if the Strings are to long, which results in the creation of an *additional* String that has a size of 131070 bytes (0xFFFF chars)!! This puts unnecessary burden on the garbage collection and is, in fact, slower than the previous version. - This is done again every time an event is serialized! - By capping the arguments a serialized and deserialized event simply isn't equal to the originally serialized event anymore, which should normally be the case. Is there any place where arguments are used as something else than Strings? I really can't see any other potential problem of my patch... -- Configure bugmail: http://bugzilla.qos.ch/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.