Opened 9 years ago

Closed 8 years ago

Last modified 4 years ago

#2250 closed patch (Fixed)

Speedup removing multiple torrents from core

Reported by: bro Owned by: bro
Priority: minor Milestone: 2.0.0
Component: Core Version: develop
Keywords: Cc:

Description

Each call to core.remove_torrent took about 1 second on my machine. Most of time 90%+ was used by writing the state file. When removing multiple torrents, the state file is now written after removing all the torrents.

https://github.com/bendikro/deluge/commit/master-core-remove-torrents

Change History (10)

comment:1 Changed 9 years ago by andar

You probably shouldn't just remove core.remove_torrent().. Keep it but just call core.remove_torrents() from it with [torrent_id].

If you remove that function you'll likely break other UIs any user written scripts.

comment:2 Changed 9 years ago by bro

Good point andar. I've readded the remove_torrent method: https://github.com/bendikro/deluge/commits/master-core-remove-torrents2

comment:4 Changed 9 years ago by bro

Fixed tests and updated with changes in develop branch: https://github.com/bendikro/deluge/commit/develop-core-remove-torrents

comment:5 Changed 9 years ago by Cas

A few notes:

  • remove_all() is a misleading function name, maybe on_remove_torrents() and also change remove_torrent() for on_remove_torrent()?
  • This import should be at top: from twisted.internet import reactor, task
  • the docstring in core.remove_torrents needs updated
  • torrentmanager.remove() method should be renamed to remove_torrents, it's not descriptive enough (easily confused with list.remove()). Also the method requires an updated docstring.
  • The type check in torrentmanager.remove() should be changed so it raises an exception if it's not a list however maybe just remove it as all remove methods should go through core for clients and we can verify that other calls in core use the torrentmanager method correctly.
  • We discussed in IRC whether the clients need to know (or care) why a torrent could not be removed, I can't remember the conclusion but do need to verify whether current clients can handle this change e.g. webui.

comment:7 Changed 8 years ago by bro

  • Milestone changed from performance to 2.0
  • Owner set to bro
  • Status changed from new to assigned
  • Version changed from master (git) to develop (git)

comment:8 Changed 8 years ago by Cas

Added to develop: [08363f28dd308]

Should be straightforward for Console and WebUI to utilise this too. I will link a branch/pull request shortly.

comment:9 Changed 8 years ago by Cas

  • Resolution set to Fixed
  • Status changed from assigned to closed

Added Console and Web code to develop [faf3f963222b1]

comment:10 Changed 4 years ago by Cas

  • Milestone changed from 2.0 to 2.0.0

Milestone renamed

Note: See TracTickets for help on using tickets.