shutils.rmtree() uses excessive amounts of memory

[jamesh]

BPO	1025127
Nosy	@tim-one
Files	shutils-rmtree.py: updated shutils.rmtree() shutil-rmtree-os-walk.diff: shutils-rmtree.py as a patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2004-10-07.21:13:28.000>
created_at = <Date 2004-09-09.13:52:40.000>
labels = ['library']
title = 'shutils.rmtree() uses excessive amounts of memory'
updated_at = <Date 2004-10-07.21:13:28.000>
user = 'https://bugs.python.org/jamesh'

bugs.python.org fields:

activity = <Date 2004-10-07.21:13:28.000>
actor = 'jlgijsbers'
assignee = 'jlgijsbers'
closed = True
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2004-09-09.13:52:40.000>
creator = 'jamesh'
dependencies = []
files = ['1401', '1402']
hgrepos = []
issue_num = 1025127
keywords = []
message_count = 6.0
messages = ['22397', '22398', '22399', '22400', '22401', '22402']
nosy_count = 3.0
nosy_names = ['tim.peters', 'jlgijsbers', 'jamesh']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1025127'
versions = ['Python 2.3']

[jamesh]

The shutils.rmtree() implementation uses an excessive
amount of memory when deleting large directory heirarchies.

Before actually deleting any files, it builds up a list
of (function, filename) tuples for all the files that
it is going to remove. If there are a lot of files,
this results in a lot of memory for a large heirarchy
(I had a Python process using 800MB in one case).

I'm not sure why it is doing things this way. It isn't
using the list to avoid recursion, so the depth of
directories it can remove is still limited by Python's
recursion limit.

Replacing _build_cmdtuple() with a generator might be a
good way to reduce the memory usage while leaving the
rest of the code unchanged.

I checked in CVS, and this issue is still present on HEAD.

[tim-one]

Logged In: YES
user_id=31435

Rewrite it using os.walk() (not os.path.walk()) with
topdown=False.

[jamesh]

Logged In: YES
user_id=146903

Attached is a Python file including a fixed up
shutils.rmtree() using os.walk(). It seems to work for me,
and should have the same error behaviour.

[jlgijsbers]

Logged In: YES
user_id=469548

Please attach changes as a patch next time. I've attached
shutils-rmtree.py as a patch this time.

I gave it a quick review and added a test to test_shutil.py
to ensure some not-very-obvious behavior (don't delete a
path passed to rmtree if it's a file) would be preserved.
The new version seems fine to me. Tim, could you take a look
at it as well?

[tim-one]

Logged In: YES
user_id=31435

I don't really have time for a thorough review. I'll note that
stuff like

func = something1
arg = something2
func(arg)

looks, to my eye, like a convoluted way to say

something1(something2)

I suppose that's to keep the onerror= gimmick working,
though.

[jlgijsbers]

Logged In: YES
user_id=469548

I just looked at my own patch again, added the _raise_err
function back in and checked it in as rev 1.33 of shutil.py.