On Jan 10, 2005, at 6:36 AM, Ceki G�lc� wrote:
At 06:23 AM 1/10/2005, Curt Arnold wrote:I haven't worked any more on this, but I do intend to do so. I thought that I would at least give a heads up that this issue isn't dead.
The current approach has a couple of drawbacks:
it incurs a synchronization for every dispatched message,
yes
it adds a sequenceCount to the LoggingEvent which breaks serialization compatibility with 1.2.x
true, but once 1.3 is released, probably no one will care.
and might be mis-interpreted as a reliable means to detect dropped messages,
This is a lopsided argument, lying beyond the realm of the rational, which I find quite disturbing. It has been brought to the table before. I have given 3 solid reasons why no rational being could ever use it for missing message detection. If required, we can go over them again. See also http://marc.theaimsgroup.com/?l=log4j-dev&m=110442953800184&w=2
Everything so far has been a summary for those who come in late. I've had users who have asked for a per-appender sequence count, it doesn't strike me as far-fetched that could be temporarily mislead by the current name but that could be addressed by renaming the field or appropriate JavaDocs.
and would report as equal serializations of the same object in different states (discussed later).
true, but exactly the same can be said about the implementation which inspects each LoggingEvent field one by one. Presumably, even that implementation will first compare events for address equality. In the current log4j processing chain the same LoggingEvent instance can be modified as it is handled by different appenders.
The same thing could not be said about a value comparison. Successive deserializations would have different object identities, so they would not return at the "if == other" check, they would only return true if they were field-by-field equal which they would not be since they could have different messages.
Since LoggingEvents have a short lifetime, the values of Object.hashCode (which I believe are similar to memory addresses) may repeat and likely in an implementation dependent fashion.
I don't understand what you are trying to say. In any case, the current LoggingEvent.hashCode implementation is damn near ideal.
I said Object.hashCode, the underlying implementation if you had called super.hashCode() within logging event. It has similar characteristics to a memory address, it would be highly unlikely that two objects that exist at the same time would have the same Object.hashCode and that could be used as an identifier. However, since LoggingEvents are typically short lived, you might see the same hashCode repeat for successive LoggingEvents, at least the probabiility of a collision would much higher. However, I guess it would be unlikely that they would repeat between garbage collections.
The approach that seems most reliable to me is to implement LoggingEvent.equals as a real value comparison and create a corresponding LoggingEvent.hashCode. That would avoid synchronization tax on every logging request and would only cost the callers of LoggingEvent.equals and LoggingEvent.hashCode.
Point well taken. The tradeoff between the creation cost for everyone and the cost for the callers of LoggingEvent.equals and LoggingEvent.hashCode should be considered with care.
I don't think any of the performance tests attempt to dispatch logging requests from multiple threads. It might be good to add some so the cost of the sync lock could be determined.
The implementation would need to be start enough so that a cloned through serialization object would compare equal to the original and that prepareForDeferredProcessing would not effect either equals or hashCode.
There would be at least two cases where the approaches give observably results. If distinct logging event with content and timestamp were received the current approach would report the messages as distinct while the new approach would report the messages as equal. This would only occur in tight loops like:
for (int i = 0; i < 20; i++) { logger.info("Hello, world"); }
The current approach would see that as twenty distinct messages while the new approach would see all messages within the same millisecond as identical.
Hence the necessity for a sequence counter.
I didn't think this scenario would be a requirement. However if it is, it might be addressed by something other than a sequence counter (Object.hashCode might be sufficient for this use).
The other scenario is that the current approach sees all deserialized copies of the same original object instance as identical.
Yes, that is the intended behavior.
log4j 1.3 adds setter methods to LoggingEvent that are used by Filters to change the message and other aspects of the logging event. If a message is sent using the SocketAppender both before and after Filter processing, with the current approach the messages would be seen as identical even though the message content, level and other aspects could be radically different.
As mentioned above, that's the risk we take by allowing post-processing by filters.
I'd love to hear your thoughts behind adding the setMessage and other mutators to LoggingEvent. It is generally a very good thing for objects to be immutable since you can eliminate a score of potential synchronization problems. In log4j 1.3, LoggingEvent's have become mutable which means that we have to think about scenarios like a LoggingEvent gets placed in an AsynchAppender then gets modified in a filter. The AsyncAppender may or may not see the new message. It is usually much easier to prevent synchronization issues by designing immutable objects than to ensure that mutable but non synchronized classes are used correctly.
My attack plan would be to first add unit tests that should check the essential features on any acceptable approach and should be passed by the current implementation, all the equals contract test cases (equals to self, equals to null), comparison to deserialized clone, hashCode invariant after prepareForDeferredProcessing. Then I would try to flesh out the implementation and corresponding tests for the value comparison implementation and report back to the list before proceeding.
Test cases. Mmmmmm.
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
