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)
Tracking
()
RESOLVED
FIXED
mozilla1.3alpha
People
(Reporter: alecf, Assigned: alecf)
References
Details
(Keywords: memory-footprint)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
gordon
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•22 years ago
|
||
adding footprint, accepting, and adding priority
Status: NEW → ASSIGNED
Keywords: footprint
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.2beta
Comment 2•22 years ago
|
||
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?
Comment 3•22 years ago
|
||
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.
Comment 4•22 years ago
|
||
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?
Comment 5•22 years ago
|
||
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.
Comment 7•22 years ago
|
||
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.
Comment 8•22 years ago
|
||
i think that the question is: why two inside necko?
Comment 9•22 years ago
|
||
Yeah, I'd like to know that, too. See bug 171150.
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.2beta → mozilla1.3alpha
Assignee | ||
Comment 10•22 years ago
|
||
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...
Assignee | ||
Comment 11•22 years ago
|
||
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
Assignee | ||
Comment 12•22 years ago
|
||
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 13•22 years ago
|
||
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?
Assignee | ||
Comment 14•22 years ago
|
||
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 15•22 years ago
|
||
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+
Assignee | ||
Comment 16•22 years ago
|
||
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.
Description
•