Closed Bug 170987 Opened 22 years ago Closed 22 years ago

ftp and ftp parsing should live in the same DLL

Categories

(Core :: Networking, defect, P2)

All
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.3alpha

People

(Reporter: alecf, Assigned: alecf)

References

Details

(Keywords: memory-footprint)

Attachments

(1 file, 1 obsolete file)

Darin sez: "FTP has all this list parsing logic which exists as a stream converter in necko.dll. however, the core FTP library lives in necko2.dll. the intention being to only load necko2.dll when a FTP link is encountered. the list parsing logic should also live in necko2.dll." However, embedding customers often want FTP but don't care about any of the other protocols in necko2.dll. There has been some talk of moving the FTP protocol code into
adding footprint, accepting, and adding priority
Status: NEW → ASSIGNED
Keywords: footprint
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.2beta
valeski was mentioning moving ftp into the core dll at some point. Since datetime and finger are now extentions, whats left in necko2? Do we still want two dlls?
we need to move viewsource out of necko2. then all that's left is FTP and gopher. so, either we move those into necko, or we push the FTP list parsing code out into necko2. i'm not sure which is the better solution, but clearly many users will never need FTP or gopher during a browsing session. i'm almost in favor of keeping necko2 and trying to lighten necko.
darin, you also mentioned factoring out other stream converters into this necko2 so. Other than nsUnknownDecoder and nsMultiMixedConv, do you see any others that we night need in the core necko library?
nsHTTPCompressConv should remain inside necko.dll (it's name is probably wrong) for some reason there's two different versions of a TXT <-> HTML converter. i'm not sure what that is all about. might be nice to get rid of one of them. looks like valeski added the nsTXTToHTMLConv module in early 2000 and someone named rph@netscape.com added the mozTXTToHTMLConv in late 1999. looks like finger, gopher, and absync depend on these. otherwise, the rest of the code in converters probably could be moved into necko2.
BenB and I were talking about some of this recently. I think he can explain some of what is going on w/ TEXT->HTML converters.
I filed about bug 171150 about ns- vs. moz-TXTToHTMLConv. Mailnews (libmime, composer) and AIM depends on the moz version. finger (IIRC) uses the ns* one. Don't know about gopher and absync.
i think that the question is: why two inside necko?
Yeah, I'd like to know that, too. See bug 171150.
Blocks: 92580
Target Milestone: mozilla1.2beta → mozilla1.3alpha
Blocks: 174807
Attached patch Move FTP into necko, from necko2 (obsolete) (deleted) — Splinter Review
I'm moving FTP out of necko2 and into necko - something embeddors have been asking for a long time. imagine that there are related mac changes in this patch as well.. the patch is pretty straight forward...
ok, here's the final version of this patch, that even builds. again, imagine there is mac build fu to go along with this.
Attachment #108045 - Attachment is obsolete: true
Comment on attachment 108055 [details] [diff] [review] Move FTP into necko, from necko2 v1.01 Hoping I can get reviews in time to get this in for 1.3alpha - the patch is really straight forward, mostly makefile changes...
Attachment #108055 - Flags: superreview?(darin)
Attachment #108055 - Flags: review?(gordon)
Comment on attachment 108055 [details] [diff] [review] Move FTP into necko, from necko2 v1.01 have you measured any impact on browser startup perf? how does this impact footprint? my original intention was to move FTP parsing out of necko.dll in order to reduce the size of necko.dll. with this change we're going the other way. is that a problem?
I haven't measured it, but this is something embeddors have been asking for - they always want ftp, so they always have to bring in necko2.dll - but they hate the idea of bringing in gopher. As for performance, the past has shown that dll consolidation has pretty minimal performance impact - this case is similar to the uconv work I did (which brought 4 rarely used DLLs into the main uconv.dll) and in that case the performance impact was negligable. (Somewhere around .5% increase in startup time, and that was moving over 400k of compiled code) given the small amount of code I'm moving over, I'm guessing the performance hit to be too small to even register with our tools/tinderbox..
Attachment #108055 - Flags: review?(gordon) → review+
Comment on attachment 108055 [details] [diff] [review] Move FTP into necko, from necko2 v1.01 ok, sounds good. hmm... so, now if we finally move viewsource out of necko (where it doesn't belong), we'd only have gopher left as part of necko2.dll... should gopher be moved to extensions perhaps (default enabled) allowing us to eliminate necko2.dll altogether?
Attachment #108055 - Flags: superreview?(darin) → superreview+
thanks folks, fix is in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: