I can confirm that the PR fixes this performance regression. I have provided more details in a comment on the PR.
Thanks again for the quick resolution. Best Regards, Richard Carl Richard Theodor Schneider schrieb am Donnerstag, 12. Januar 2023 um 00:07:07 UTC+1: > > First of all, you're really on your own as far as Python2 is concerned. > > Yes, I know and understand this. The intention behind this was merely to > provide another data point, as Nils noticed. > > > It's not clear from your message whether you saw any difference from > > python3 vs "sage --python". > > Could you clarify? > > The time for both `sage` and `sage --python` was >7s for both in the sage > 9.x-case, while the native usage of fpylll from within python3 ran in the > expected time (same order of magnitude as sage8.x and py2). The three test > cases for the recent version are in line 35-40, 42-47 and 49-53 of the log. > > > Could you please also say what versions of libfplll you were trying. > > In the case where everything worked fine, the versions are: > sage: SageMath version 8.9, Release Date: 2019-09-29 > py: 2.7.18 (default, Apr 19 2020, 21:45:35) [GCC 9.3.0] > fpylll: 0.5.1dev > fplll: 5.3.2 > > In the case where sage was slow, but py3 performed as expected, the code > ran with these versions: > sage: SageMath version 9.6, Release Date: 2022-05-15 > py: 3.10.5 (main, Jun 6 2022, 12:05:50) [GCC 11.3.0] > fpylll: 0.5.7 > fplll: 5.4.2 > > > I'll apply the patch from the PR to verify whether it resolves the problem > and report back (probably in the afternoon for UTC+1). > > Best Regards and thanks for the help so far, > Richard > > martinr...@googlemail.com schrieb am Mittwoch, 11. Januar 2023 um > 23:30:56 UTC+1: > >> Thanks, it’s this function and a similar one the other way, i.e. an >> FPyLLL issue when used inside Sage: >> >> ``` >> cdef int assign_mpz(mpz_t& t, value) except -1: >> """ >> Assign Python integer to Z_NR[mpz_t] >> """ >> if isinstance(value, int) and PY_MAJOR_VERSION == 2: >> mpz_set_si(t, PyInt_AS_LONG(value)) >> return 0 >> if isinstance(value, long): >> mpz_set_pylong(t, value) >> return 0 >> if have_sage: >> from sage.rings.integer import Integer >> if isinstance(value, Integer): >> value = long(value) >> mpz_set_pylong(t, value) >> return 0 >> ``` >> >> Per your profiler output we’re calling this 2*250*250 times, i.e. once to >> convert in and once to convert out. >> >> This should be fixed with this PR: >> >> https://github.com/fplll/fpylll/pull/239 >> >> I’ll cut a new release and update https://trac.sagemath.org/ticket/34870 >> >> Cheers, >> Martin >> >> On Wed, Jan 11 2023, Nils Bruin wrote: >> > Of course going back to py2 is not a solution but the data point is >> > indicative that a regression was introduced, and that would be worth >> > solving. It looks to me the following code is what exhibits the >> problematic >> > behaviour. Run on a recent sage (9.7 or so): >> > >> > sage: from fpylll import * >> > sage: FPLLL.set_random_seed(0) >> > sage: dim = 250 >> > ....: bits = 3 >> > ....: A = IntegerMatrix.random(dim, "uniform", bits = bits) >> > sage: %prun B = LLL.reduction(A) >> > >> > (with profiling to see if anything comes to light): >> > >> > 19250003 function calls (18500003 primitive calls) in 6.983 seconds >> > >> > Ordered by: internal time >> > >> > ncalls tottime percall cumtime percall filename:lineno(function) >> > 125000 0.665 0.000 3.127 0.000 <frozen >> > importlib._bootstrap>:921(_find_spec) >> > 375000/125000 0.612 0.000 6.368 0.000 <frozen >> > importlib._bootstrap>:1022(_find_and_load) >> > 1 0.548 0.548 6.983 6.983 <string>:1(<module>) >> > 125000 0.504 0.000 1.525 0.000 <frozen >> > importlib._bootstrap_external>:1536(find_spec) >> > 375000 0.389 0.000 0.719 0.000 <frozen >> > importlib._bootstrap>:179(_get_module_lock) >> > 375000/125000 0.326 0.000 5.472 0.000 <frozen >> > importlib._bootstrap>:987(_find_and_load_unlocked) >> > 625000 0.268 0.000 0.668 0.000 <frozen >> > importlib._bootstrap_external>:126(_path_join) >> > 375000 0.264 0.000 0.328 0.000 <frozen >> > importlib._bootstrap>:125(release) >> > 375000 0.261 0.000 0.325 0.000 <frozen >> > importlib._bootstrap>:100(acquire) >> > 625000 0.249 0.000 0.331 0.000 <frozen >> > importlib._bootstrap_external>:128(<listcomp>) >> > 375000 0.206 0.000 0.273 0.000 <frozen >> > importlib._bootstrap>:71(__init__) >> > 125000 0.200 0.000 0.200 0.000 {built-in method posix.stat} >> > ........... >> > >> > Nevermind that the times don't add up, we see a profile that is >> completely >> > dominated by import stuff! So it looks to me we may have ended up with >> an >> > import in some fairly tight loop. Regardless of whether this is the >> actual >> > source of the problem observed here, I think it's problematic in its >> own >> > right. >> >> >> -- >> >> _pgp: https://keybase.io/martinralbrecht >> _www: https://malb.io >> _prn: he/him or they/them >> >> -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/fa26fce0-b243-4251-b2fe-4c676158382en%40googlegroups.com.
