fix: Reduce logging system overhead #3416
Conversation
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
jjerphan
force-pushed
the
fix/libsolv-database-log-overhead
branch
2 times, most recently
from
a2f847e to
2c8b4ba
Compare
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
There was a problem hiding this comment.
The failure of test_remove_orphaned in this PR is weird, I've restarted the CI on current main to check if something has changed in the dependencies of xtensor-python. If it does not fail, then the test can be considered as flaky since no change in this PR is related to this.
UPDATE: the failure also occurs on main and branch 1.x, so something has changed in the ecosystem and the test must be fixed accordingly.
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Johan Mabille <johan.mabille@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
A dry run takes slightly less than 10 seconds. Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
See: https://github.com/mamba-org/mamba/blob/1.x/libmamba/src/core/pool.cpp#L72 Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
I was hesitating between the previous solution and this one (which is not really ideal either) but which at least does not leak the `Context`. Friend function could be used but we get this kind of issue, but I am currently meeting this problem: https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2174 Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
This reverts commit f1f41b7.
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
15 seconds is not sufficient as shown on the CI. Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
micromamba/tests/test_create.py
Outdated
| @@ -143,6 +143,24 @@ def test_env_lockfile_different_install_after_create(tmp_home, tmp_root_prefix, | |||
| helpers.install("-p", env_prefix, "-f", install_spec_file, "-y", "--json") | |||
|
|
|||
|
|
|||
| # Only run this test on Linux, as it is the only platform where xeus-cling | |||
| # (which is part of the environment) is available. | |||
| # @pytest.mark.timeout(60) | |||
There was a problem hiding this comment.
I do not currently understand why this timeouts on the CI, whereas my manual test takes 10 seconds locally.
There was a problem hiding this comment.
Let me check when running the Python test suite.
There was a problem hiding this comment.
It actually took 93.58s.
There was a problem hiding this comment.
Hum, and when it hangs (without the fix) how much time does it take before finishing (with dry run)? (If it's still hanging after let's say 200 sec we can set the timeout limit to maybe 100sec?).
There was a problem hiding this comment.
On main, it hangs for about one hour.
If I set the level to 0 (which entirely removes logs), it takes less than 20 seconds.
There was a problem hiding this comment.
Ok so let's set 100 for the timeout.
There was a problem hiding this comment.
Yes, let me just look at the default value of verbosity in 1.x to know whether the value passed as level was 3 or something less as it could still introduce a performance regression.
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
| // Instead use something like: | ||
| // const int level = Context().output_params.verbosity - 1; | ||
| const int level = 0; | ||
| ::pool_setdebuglevel(pool().raw(), level); |
There was a problem hiding this comment.
In 1.x we were doing:
pool_setdebuglevel(pool().raw(), Context::instance().output_params.verbosity - 1);
when verbosity >= 3
Otherwise pool_setdebuglevel wasn't called. That means that it was set to something in libsolv internals not to the default value of verbosity in mamba.
There was a problem hiding this comment.
The default value of libsolv's pool->debugmask is SOLV_DEBUG_RESULT.
If we call it with 0 we only &=SOLV_DEBUG_TO_STDERR it. Not sure if it causes a problem in practice, but let's thus not call it for now at all.
This is handled by another PR. See: mamba-org#3417 Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
|
The added test was skipped here, so we should have waited for #3417 being merged before merging this... |
Partially addresses #3415.
This fixes the regression which has been observed in several release candidates of 2.0.0 by restoring the behavior of
1.x.@Hind-M and I believe that we should think of the UX of the verbosity level handling as proposed in #3415 in another PR, especially given this work would necessitate much more work with potentially change to public APIs.