Ticket #11716 (closed enhancement: fixed)

Opened 22 months ago

Last modified 21 months ago

Remove twisted.persisted.styles import

Reported by: saraedum Owned by: jason
Priority: critical Milestone: sage-4.7.2
Component: performance Keywords: sd32 start-up time startup
Cc: Work issues:
Report Upstream: N/A Reviewers: Mike Hansen
Authors: Julian Rueth Merged in: sage-4.7.2.alpha3
Dependencies: Stopgaps:

Description (last modified by leif) (diff)

The import of twisted.persisted.styles takes a significant amount of time on sage startup: 

$ time ./sage -startuptime|grep twisted.persisted.styles:
 twisted.persisted.styles: 0.093 (sage.all)

real    0m1.422s

Most functionality from that module seems not to be used in sage. The attached patch removes everything but the needed functionality: 

$ time ./sage -startuptime|grep twisted.persisted.styles:

real    0m1.280s

Apply only trac_11716_twisted_persisted_styles.proper.patch Download to the Sage library.

Attachments

trac_11716_twisted_persisted_styles.patch Download (3.0 KB) - added by saraedum 22 months ago.
trac_11716_twisted_persisted_styles.proper.patch Download (3.0 KB) - added by leif 21 months ago.
"Proper" Mercurial changeset replacement patch.

Change History

comment:1 Changed 22 months ago by mhansen

  • Reviewers set to Mike Hansen

I like the idea of this, but I think that the methods should be moved into say "sage.misc.fpickle" instead of "sage.misc.cachefunc".

Changed 22 months ago by saraedum

comment:2 Changed 22 months ago by saraedum

Good point. I was actually unsure where to put it. I fixed the patch accordingly. I'm waiting for the doctests to finish now.

comment:3 Changed 22 months ago by saraedum

  • Status changed from new to needs_review

Passed the doctests against 4.7.2.alpha2.

comment:4 Changed 22 months ago by robertwb

  • Status changed from needs_review to positive_review

That one was at the top of my hit list too--I'm relieved it turned out to be so painless.

comment:5 Changed 22 months ago by jdemeyer

saraedum: please add your real name as Author on this ticket. It would also be good to put yourself on  http://trac.sagemath.org/sage_trac/#AccountNamesMappedtoRealNames.

comment:6 Changed 22 months ago by jdemeyer

  • Priority changed from minor to critical
  • Component changed from misc to performance

comment:7 Changed 22 months ago by was

  • Authors set to Julian Rueth

I added saraedum's real name to the Author on the ticket at the wiki.

comment:8 Changed 22 months ago by was

  • Keywords sd32 added

comment:9 Changed 22 months ago by leif

  • Keywords start-up time startup added
  • Description modified (diff)

comment:10 Changed 21 months ago by leif

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.7.2.alpha3

Changed 21 months ago by leif

"Proper" Mercurial changeset replacement patch.

comment:11 Changed 21 months ago by leif

  • Description modified (diff)
  • Summary changed from remove twisted.persisted.styles import to Remove twisted.persisted.styles import

I've attached a *.proper.patch, which is identical except that I removed the "garbage" before "# HG changeset patch", i.e., I deleted the first line "exporting patch:", since Jeroen's current merger rejects such patches.

For now, please make sure all your patches start with "# HG changeset patch", i.e., have it on the first line without any preceding messages or whatever.

I've relaxed that in my version of the merger, but Jeroen and maybe others are likely to use his more restrictive one.

Note: See TracTickets for help on using tickets.