Opened 5 years ago

Closed 10 months ago

#23327 closed defect (invalid)

Small regression with MPIR-3.0.0

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords:
Cc: vdelecroix, jpflori, fbissey, dimpase Merged in:
Authors: Reviewers: Matthias Koeppe, Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #32549 Stopgaps:

Status badges

Description (last modified by jdemeyer)

#22493 has caused a significant regression for padic_base_leaves.py (this was discovered while investigating #23284, it is not necessarily limited to that file).

Before #22493: [142 tests, 178.75 s]

After #22493 and #23209: [142 tests, 181.61 s]

(in both cases, this was a best-out-of-10 timing but the variance is small enough that this is statistically very significant)

Change History (12)

comment:1 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 5 years ago by jdemeyer

  • Cc vdelecroix jpflori fbissey dimpase added

comment:3 Changed 5 years ago by dimpase

IMHO 10% performance decrease is not too bad. As well, it's unclear whether it's limited to a particular compiler/OS/hardware combo, or not. One can also say that MPIR has added more assembly codes, and I guess one can as well find a combo that shows a performance boost.

comment:4 Changed 5 years ago by dimpase

  • Priority changed from blocker to major

I really cannot see this as the reason not to upgrade MPIR. Consider using ODK funds to get a new CPU where new MPIR shines :-)

comment:5 Changed 5 years ago by jdemeyer

  • Priority changed from major to blocker

Maybe we can accept a 10% performance regression, but certainly not without further investigation. The problem may be much worse than it appears to be on the surface.

comment:6 Changed 5 years ago by dimpase

in any event, please attach mpir-3.0.0.p0.log of the installation where you see this slowdown.

comment:7 Changed 5 years ago by mmezzarobba

Is that the same issue as #23209?

comment:8 Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Priority changed from blocker to major
  • Summary changed from Regression with MPIR-3.0.0 to Small regression with MPIR-3.0.0

Right, the regression is mostly explained by #23209. But even then, there remains a smaller regression of about 2%.

comment:9 Changed 10 months ago by mkoeppe

  • Dependencies set to #32549
  • Milestone changed from sage-8.0 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

Outdated after mpir removal in #32549

comment:10 Changed 10 months ago by mmezzarobba

  • Status changed from needs_review to positive_review

comment:11 Changed 10 months ago by slelievre

  • Reviewers set to Matthias Koeppe, Marc Mezzarobba

comment:12 Changed 10 months ago by mkoeppe

  • Resolution set to invalid
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.