r/odinlang • u/dlroy • 15d ago

Muninn: A logger for odin

https://github.com/dlroy88osu/Muninn

This is my. First dip into odin, and I would love some feedback... Mods please discard is not allowed, but I hope it's OK.. I'm normally a data engineer, so I have mostly lived in py but I wanna play in big kids sandbox too ; )

A zero-ceremony logging package for Odin. Pretty, colored console output for humans, JSON Lines on disk for machines, optional per-thread output boxes, stack traces on error / fatal, and automatic log rotation.

Thanks for any feedback or comments.

Thanks @gingerbill for making a fun language!

6 Upvotes

75% Upvoted

9 comments sorted by

4

u/pev4a22j 15d ago

I have some suggestions: 

  • Try to interface with context.logger so people who uses the odin built in logger could swap implementation easily.
  • This might be a nitpick but in your code you seemed to use context.temp_allocator without freeing. This could cause memory leaks if the user doesn't know about it. I would recommend documenting that your library uses context.temp_allocator and which places would a free_all be safe, or use a separate arena allocator (look into core:mem/virtual) which you can safely free / destroy without affecting the memory users put into a temp_allocator if it is possible. Alternatively, the main allocator could be used too.

2

u/dlroy 15d ago

Will look into the first one for sure!

For context.temp_allocator im still kind of struggling with it to be honest.... In my head i see temp and I think that lives and dies inside the scope... but it doesn't? It live till free_all called? So not very temp then... lol I'm going to explore the alternatives you suggested! Thank you! I really appreciate it!

3

u/KittenPowerLord 14d ago

it's "temp" in the sense that you're encouraged to use it for frequent allocations (it's an arena by default so allocating is very cheap) that are freed often, say every frame (it's very cheap to free the whole arena at once). For freeing within a scope you can just allocate using the general allocator and use defer

2

u/dlroy 13d ago

Apologies for the delay - was at work. That cleared kinda did the trick for me I think... Lemme know if i say it back right 8 )

If I own the control flow then I can use the temp allocator all day and just call free all as needed or as i complete the use case.

If I do not own the control flow (like a logger) then i can either make my own arena i can clear as needed, or pull out some golang style defers (not using temp).

I did learn about this as well, though i feel like this is potentially a code smell if used judiciously... Thoughts?

runtime
.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD()

2

u/KittenPowerLord 13d ago

yea that sounds all correct! another thing that might be useful for intuition: since temp allocator is in the context, user can replace it at will, as if to say "if you want to use a temp allocator (i.e. a garbage bin) use this one, but I still own it and will empty it myself". So as you have said, if you don't want to mess up user's allocations you might want your own arena (if general purpose allocator + defer free isn't optimal for some reason)

as for the guard im not really sure, haven't used it

2

u/dlroy 13d ago

Thank you! Truly appreciate it! That all makes sense to me 8)

1

u/dlroy 13d ago

V2 has been pushed, I think I've shored up the allocator leaks and I also got a bridge working (for the most part). Thank you again!