Merge lp://staging/~cjwatson/meliae/test-sys-getsizeof into lp://staging/meliae

Proposed by Colin Watson
Status: Merged
Approved by: John A Meinel
Approved revision: 203
Merged at revision: 202
Proposed branch: lp://staging/~cjwatson/meliae/test-sys-getsizeof
Merge into: lp://staging/meliae
Prerequisite: lp://staging/~cjwatson/meliae/tox
Diff against target: 281 lines (+63/-81)
2 files modified
meliae/_scanner_core.c (+4/-4)
meliae/tests/test__scanner.py (+59/-77)
To merge this branch: bzr merge lp://staging/~cjwatson/meliae/test-sys-getsizeof
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+378244@code.staging.launchpad.net

Commit message

Test scanner.size_of against sys.getsizeof.

Description of the change

The exact details of object sizes are difficult to maintain across Python versions, and it's all too easy for bugs to go undetected due to parallel implementation bugs in the code under test and the test suite.

Python 2.6 introduced `sys.getsizeof`, so in all but a few special cases it makes more sense to test against that.

In the process of doing this, I found and fixed two bugs in `scanner.size_of`: the size of Unicode objects was off by one, and the size of statically-allocated type objects incorrectly included the size of a GC head.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

IIRC when this was written either sys.getsizeof was inaccurate or didn't exist. If it is strongly trusted now, then I'm not sure if we need scanner.size_of. Maybe it avoids allocating a PyLong to do the math?

Revision history for this message
Colin Watson (cjwatson) wrote :

meliae having its own version certainly allows for avoiding sys.getsizeof/__sizeof__ in a few more cases and I expect reduces the probability of it needing to do allocations. I don't think sys.getsizeof makes any particular guarantees about avoiding allocations, and it'll certainly make some in the course of calling __sizeof__.

(I also generally thought it best to minimise change for the purposes of this porting work; changing the tests to be more robust was one thing, but ripping out a chunk of the scanner seemed a bit more intrusive.)

Revision history for this message
John A Meinel (jameinel) wrote :

Small steps, though we should consider whether having our own sizeof is worth the tradeoff.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: