Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#11716 closed enhancement (fixed)

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: Merged in: sage-4.7.2.alpha3
Authors: Julian Rueth Reviewers: Mike Hansen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by leif)

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 to the Sage library.

Attachments (2)

trac_11716_twisted_persisted_styles.patch (3.0 KB) - added by saraedum 8 years ago.
trac_11716_twisted_persisted_styles.proper.patch (3.0 KB) - added by leif 8 years ago.
"Proper" Mercurial changeset replacement patch.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years 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 8 years ago by saraedum

comment:2 Changed 8 years 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 8 years ago by saraedum

  • Status changed from new to needs_review

Passed the doctests against 4.7.2.alpha2.

comment:4 Changed 8 years 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 8 years 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 8 years ago by jdemeyer

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

comment:7 Changed 8 years 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 8 years ago by was

  • Keywords sd32 added

comment:9 Changed 8 years ago by leif

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

comment:10 Changed 8 years ago by leif

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

Changed 8 years ago by leif

"Proper" Mercurial changeset replacement patch.

comment:11 Changed 8 years 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.