[EMAIL PROTECTED]">excellent! w/ out having seen the code, I bet that 7% comes from the removal of thread context switching (it's now on the main thread), and subsequent removal of all the proxy object use (heavy on event queues). I bet on a large download the savings is more like 1-3% though as all the context switching and proxy event handling is heaviest in negotiating the control connection (versus the data connection). Nice work!I have been hacking on FTP for the last two weeks (prior to a short but well spent vacation). The mission was to remove the threadpool and make FTP asynchronous. I am about a week away from landing assuming that I do not run into any major problems. During this time, I hope to find as may problems as possible. If you are so inclined, feel free to help me out and pull these changes.
A quick look at the performance delta is about 7% for directory listing download (not tree/rdf building) favoring the new implementation. I hope to gather some more in-depth data later.
[EMAIL PROTECTED]">Nice win!How to update:
cvs up -rDOUGT_FTP_12122000_BRANCH mozilla/netwerk/protocol/ftpTags:
FTP Base tag: DOUGT_FTP_12122000_BASE
FTP Branch tag: DOUGT_FTP_12122000_BRANCHDiffs:
cvs diff -u -rDOUGT_FTP_12122000_BASE -rDOUGT_FTP_12122000_BRANCH mozilla/netwerk/protocol/ftpSocket Transport Patch:
Youll, probably want to apply the patches in this bug too:
http://bugzilla.mozilla.org/show_bug.cgi?id=63565Without these changes, you will leak socket transports. I should be checking in this fix in the next day or so.
FTP Changes include:
- Changes to make FTP asynchronous (eg. be driven by the socket transport).
- Moved status codes from public idl to ftpCore.h
- Reworked connection cache to store the ftp channel instead of the underlying socket. This was done so that we could maintain an open pipe to the server. The previous implementation did not use Async calls so there was no tie between the channel and transport. AsyncRead holds an AddRef to the stream listener. There is not a way to break this without closing the pipe.
- Remove all the AsyncEvent and nsISupport Proxy code. This is obsolete since we are asynchronous.
[EMAIL PROTECTED]">didn't know you could do this; cool! I bet this didn't exist when I put that hash table in.
- Removed lock from nsFTPHandler which protected the connection hashtables. Instead, I created the hashtables thread safe.
[EMAIL PROTECTED]">Nice!
- Removed ThreadPools
[EMAIL PROTECTED]">Agreed. nice move.
- Remove the ConnectionCache interface. Static methods are better for this kind of thing because we are in the same component and we do not have to hold references to the protocol handler.
[EMAIL PROTECTED]">agreed. I wonder if Available() is of any use?
- Create a new object nsFtpControlConnection which will wrap the control pipe so that we can (a) cache it and (b) be able to replace the observer without closing the socket.
- Renamed mConnThread to mFtpState
- Removed Available() call in OnDataAvailable. We should just the byte count that was passed to us.
[EMAIL PROTECTED]">Understand FTP servers, c'mon. that'll never happen no matter how many log statements you drop in :).
- Implemented nsFtpState::OnStopRequest so that it cancels us. This is to avoid leaks when a problem occurs before the control connection is established.
- More PR_LOG's so that we can understand what the heck is going on! :-)
[EMAIL PROTECTED]">Are you going to do this whitebox w/ programmatic timing (clock checks in the download code?)?
- A whole bunch more... :-)
Known Things To Do:
- Need some perf verification
[EMAIL PROTECTED]">Agreed. We need to really regress these changes before dropping them in.
- Need some mac love
- Better caching and management of control connections
- Check for regressions
Jud
PS - doug and I are going to go over these changes line-by-line this coming Monday.
[EMAIL PROTECTED]">
