Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#6027 closed defect (fixed)

[with patch, positive review] get_memory_usage() sucks performance wise on OSX

Reported by: mabshoff Owned by: mabshoff
Priority: critical Milestone: sage-4.1
Component: porting Keywords:
Cc: rdingman Merged in: sage-4.1.alpha0
Authors: Ryan Dingman Reviewers: Nick Alexander
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

----------------------------------------------------------------------
| Sage Version 3.4.2, Release Date: 2009-05-04                       |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: timeit('get_memory_usage()')
5 loops, best of 3: 486 ms per loop

This causes sage/rings/tests.py to take forever in -long doctesting mode.

See http://blog.kuriositaet.de/?p=257 for a more efficient way to get the current memory used.

Cheers,

Michael

Attachments (2)

trac_6027_get_memory_usage_darwin.patch (7.9 KB) - added by rdingman 11 years ago.
trac_6027-osx104.patch (2.1 KB) - added by was 10 years ago.

Download all attachments as: .zip

Change History (11)

Changed 11 years ago by rdingman

comment:1 Changed 11 years ago by rdingman

  • Cc rdingman added
  • Summary changed from get_memory_usage() sucks performance wise on OSX to [with patch; needs work] get_memory_usage() sucks performance wise on OSX

I've added a patch to get the memory usage of Sage on Darwin without spawning top and parsing the output. With this patch, get_memory_usage() will still report the same result that top reports for VSIZE (the link above doesn't do this).

Before patch:

sage: timeit('get_memory_usage()') 5 loops, best of 3: 156 ms per loop

After patch:

sage: timeit('get_memory_usage()') 125 loops, best of 3: 2.62 ms per loop

This has only been tested on OS X 10.5, Intel, 64-bit. It will likely work for PPC and 32-bit (still 10.5), but I'm not sure about 10.4 and earlier. I don't have access to hardware right now to test (and fix) this patch on all these other configurations. I'd be happy to finish up this patch if someone has machines to test on.

comment:2 Changed 11 years ago by rdingman

Also, for other platforms calling the underlying darwin_memory_usage() will just raise NotImplementeError?. I also have not tested this patch on non-darwin to make sure the c code conditionally compiles correctly and that the behavior is reasonable.

comment:3 Changed 11 years ago by mabshoff

  • Summary changed from [with patch; needs work] get_memory_usage() sucks performance wise on OSX to [with patch; needs review] get_memory_usage() sucks performance wise on OSX

I have no doubt it will compile, so mark it for "needs review". Otherwise no one will look at this any time soon since those tickets tend to stay rotten in trac ;)

Once I have made it home I will look at this. I have some analog code for Solaris more or less ready to go and will extend this code as needed then.

Cheers,

Michael

comment:4 Changed 10 years ago by was

  • Milestone changed from sage-4.0 to sage-4.0.1

This isn't critical for 4.0. This would be very nice to get into 4.0.1.

comment:5 Changed 10 years ago by ncalexan

  • Summary changed from [with patch; needs review] get_memory_usage() sucks performance wise on OSX to [with patch, positive review] get_memory_usage() sucks performance wise on OSX

Applies clean, tested fine on sage.math. After patch on OS X 10.5:

sage: sage.misc.getusage.get_memory_usage()
141.22265625
sage: %timeit sage.misc.getusage.get_memory_usage()
100 loops, best of 3: 5.24 ms per loop
sage: sage.misc.darwin_utilities
sage.misc.darwin_utilities
sage: sage.misc.darwin_utilities.darwin_memory_usage()
148082688L
sage: %timeit sage.misc.darwin_utilities.darwin_memory_usage()
100 loops, best of 3: 5.22 ms per loop

comment:6 Changed 10 years ago by ncalexan

  • Authors set to Ryan Dingman
  • Reviewers set to Nick Alexander

comment:7 Changed 10 years ago by rlm

  • Merged in set to sage-4.1.alpha0
  • Resolution set to fixed
  • Status changed from new to closed

Changed 10 years ago by was

comment:8 Changed 10 years ago by rlm

Positive review on William's additional patch, which is merged in sage-4.1.alpha1.

comment:9 Changed 10 years ago by rdingman

If someone has machines with OSX <= 10.4 that I can access I could port the code I wrote.

Note: See TracTickets for help on using tickets.