search

rs / zerolog

Zero Allocation JSON Logger
MIT License
10.4k stars 567 forks source link

Add fields order #550

Closed madkins23 closed 4 months ago

madkins23 commented 1 year ago

After I did this work I saw PR 170 which is apparently languishing. My bad for not checking first. On the other hand this PR currently has no merge issues and I have time to respond to comments.

rs commented 6 months ago

Could you please make this change not allocating additional memory on each write if the feature is not used?

madkins23 commented 6 months ago

Good idea. That also cleaned up the code a lot. Not sure what I was thinking when I did all that before. :-(

I stored a map[string]bool on the ConsoleWriter to speed up subsequent calls. It won't get created until the first call to orderFields so it's just a wasted pointer otherwise, and ConsoleWriter shouldn't be created a bunch of times unless I'm missing something.

BTW, there's another similar situation with FieldsExclude at the beginning of ConsoleWriter.writeFields that could be solved the same way. The current solution iterates over w.FieldsExclude for each field which feels like O(n^2). I don't mind writing the PR if you agree @rs.

Both of those last two notes assume that ConsoleWriter fields don't get changed over time. If that is not true then I need to remove that tweak on this PR. The w.FieldsExclude issue might still benefit from being converted to a map[string]bool at the beginning of ConsoleWriter.writeFields.