Fix log level filtering in multilog environments

[izolyomi]

Fixes issue described in #26

[RegenJacob]

I'll take a look at it when I'm at home.

But currently ci is failing could you maybe take a look at? @izolyomi

[izolyomi]

I'm sorry for the incomplete PR, I've checked the error, it should work in usual setups. I don't have too much GitHub Ci/Actions experience, but it seems to be caused by some exotic CI configuration disabling std features of the log crate.
I'm not sure in which environments this crate is supposed to run in, so @RegenJacob please take a look at it yourself as well.

[RegenJacob]

I'm sorry for the incomplete PR, I've checked the error, it should work in usual setups. I don't have too much GitHub Ci/Actions experience, but it seems to be caused by some exotic CI configuration disabling std features of the log crate. I'm not sure in which environments this crate is supposed to run in, so @RegenJacob please take a look at it yourself as well.

Don't stress yourself this is just small hobby project. I'm really glad that people contribute.

I can confirm that it runs and builds on normal setup. It's just a bit weird that CI fails since I didn't set it up to be a no std build environment (but that's not important right now).

Could you maybe add some documentation and include it in the hello example? (if the PR is ready of course)

And thanks for your PR!

[izolyomi]

Thanks for being open and cooperating, I've just had the chance to have another round with this PR.
I've checked what you meant by the hello example, but I think this PR is waaay simpler than you'd think, it's just bugfix for log level filtering. There's no change in the public API or extra documentation needed and all the examples and tests run without modification. I think it should be considered reasonably ready.

Meanwhile I also took a look at some of your implementation details and added 2 minimal changes for optimization and cleanup. I've just added them in a new commit into this PR. Hm, maybe it would have been luckier to discuss it in a separate PR?

1. Removed the extra hashbrown dependency since the standard HashMap took the hashbrown implementation since a while.
2. Used a ring buffer, i.e. VecDeque instead of Vec to store log records. This way, truncating the log entry list to max_log_length using drain() does not have to shift all the elements of the vector in memory.

Regarding the CI problem, I'm not sure if I'm even able to see the project settings, how could we solve this together?

[RegenJacob]

I'm a bit unsure about VecDeque as it could hurt performance a bit but this will need some benchmarking.
Probably better for another PR.

The rest seems good!

CI is configured in .github/workflows/rust.yml but it should work as it's just Linux with a normal configuration.

However if you move the VecDeque changes into another PR I'll merge this.

[izolyomi]

Thanks for the fast reply!

I'm a bit unsure about VecDeque as it could hurt performance a bit

Theoretically, keeping a Vec instead of VecDeque would hurt performance a lot. Both are backed up by an array, but while Vec prefers keeping elements in a contiguous slice at the cost of poor performance at element removal, VecDeque prefers fast removal of the first elements instead, just like done in egui_logger's logs.drain(..limit) in ui.rs:138 and gets slow at explicit https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.make_contiguous which is never called in egui_logger. See the standard documentation explicitly suggesting to use VecDeque in such cases: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.remove.

However, I haven't done the proper homework to show reasonable benchmarking results to prove these claims, so I don't want to force this change on you.

Regarding the CI, I'll try to somehow fix this in my fork and come back with the results.

[RegenJacob]

I tried to do some naive Profiling with puffin:

1000 Log entries with Vec

10,000 Log entries with Vec

1000 Log entries with VecDeque

10,000 Log entries with VecDeque

So it seems to be about the same for the rendering. Performance issue from #26 still apply.

[RegenJacob]

I think after postponing the Vec Changes and introducing an std-feature to the crate to fix CI (and wasm in the future).

This will be good to go!

[RegenJacob]

( oops it seems that i forgot to add those 2 comments?)

[izolyomi]

Thanks for the benchmark. Ok, I've moved back to Vec.
I've also took a turn with feature-s, but then inspired from the other PR #31 and better solved the logger lifetime issues without needing std features of the log crate.