Closed
Bug 378629
Opened 18 years ago
Closed 18 years ago
SSL file uploads settle into oscillating pattern with very small packets
Categories
(Core :: Networking, defect, P2)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: bzbarsky, Assigned: KaiE)
References
Details
(Keywords: dataloss, fixed1.8.1.5, regression, Whiteboard: corruption)
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
rrelyea
:
review+
dveditz
:
approval1.8.1.5+
|
Details | Diff | Splinter Review |
See bug 356470. This causes two problems: 1) It's suboptimal from the point of view of performance, I would guess 2) It tickles a bug in Apache servers. While Apache has released a fix, there's no guarantee that everyone is upgrading. We can push fixes a lot better than Apache can, given update. Oh, and this is apparently a branch regression too. Still waiting on feedback on whether the patch I posted in bug 356470 actually helps or whether we need changes to the secureWrite implementation too.
Flags: blocking1.9?
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Comment 1•18 years ago
|
||
If an objective is to speed up uploads, then you should also increase the minimum write size to a much larger value for all writes except the last. I might suggest 64KB. The reason is that writes of any amount less than the TCP transmit window size (a 16 bit value) are subject to Nagle delays.
Most comments on bug 356470 are irrelevant to this bug until bug 356470 comment 44. (Regression is narrowed down in bug 356470 comment 15 and bug 356470 comment 19.) Also, I'm pretty sure the OS for this should be Windows only, not Linux.
![]() |
Reporter | |
Comment 3•18 years ago
|
||
Ah, so Nelson reminded me that the function we really care about is ssl_SecureSend, not nsSSLThread::requestWrite. And indeed, ssl_SecureSend is getting the alternating sizes when it's called, even with my patch. I traced through it, and nsSSLThread::requestWrite seems to also have a 4kb buffer that it's filling (but not really!), causing the problems. We shouldn't really even need changes to the various streams, assuming this code is fixed, I think. The pattern is the following: 1) We call requestWrite with 4096 bytes. It's in the ssl_writing_done state. 2) si->mThreadData->mSSLResultRemainingBytes is some small number N (8 bytes, or 1 byte, or 163 bytes, whatever). 3) It sets the state to ssl_idle and returns N 4) We call requestWrite with 4096 - N bytes. It's in the ssl_idle state. It copies 4096 - N bytes into its buffer, and sets state to ssl_pending_write. 5) It returns -1 and sets the error to WOULD_BLOCK. 6) We call requestWrite with 4096 bytes. It's in the ssl_writing_done state. 7) si->mThreadData->mSSLResultRemainingBytes is 4096 - N 8) It sets state to ssl_idle and returns 4096 - N. I guess it copied that data in on the previous call, eh? 9) We call requestWrite with N bytes. It's in the ssl_idle state. It copies N bytes into its buffer and sets state to ssl_pending_write. 10) It returns -1 and sets the error to WOULD_BLOCK After that we go back to step 1. What would it take for this code to just go ahead and fill the buffer before trying to put it on the wire? Kai, any ideas?
![]() |
Reporter | |
Comment 4•18 years ago
|
||
As far as OS goes, we get the oscillating behavior on all platforms; it might be that our headers are slightly different on different platforms (e.g. different UA string) leading to different oscillation sizes).
OS: Linux → All
Hardware: PC → All
Assignee | ||
Comment 5•18 years ago
|
||
Boris,
yes, nsSSLThread::requestWrite is using its own buffer to collect the pending data, so it can be used on the other thread.
But that buffer is not limited to 4K. Individually for any particular socket, the buffer will grow to the maximum value the caller attempted to write in a single call.
You are proposing the SSL thread could:
> go ahead and fill the buffer before
> trying to put it on the wire
But how would the implementation of requestWrite know, whether or not the caller will request to write more data? (The caller might as well be finished after this chunk and request a read next.)
Are you proposing to introduce a timer / a delay?
Should the SSL thread wait for a small amount of time, while the implementation of requestWrite could potentially add more data to the buffer?
Sounds tricky.
Alternatively, could this get optimized at the caller side?
The caller could know it has more than (4096-N) bytes queued, and therefore could always call requestWrite( min(4096, queuesize)) ?
![]() |
Reporter | |
Comment 6•18 years ago
|
||
> But how would the implementation of requestWrite know, whether or not the > caller will request to write more data? Because on the previous call the caller tried to write more and we didn't consume all of it? For example, if the caller attempts to write an amount that _would_ fit in the buffer but doesn't because of whatever our leftover state is (as here), could we consume nothing on that call? And then consume all the data the caller > Alternatively, could this get optimized at the caller side? Not really; the caller has been asked to write 4096 bytes to the socket (by the HTTP code); it's just doing that. It can write it a bit at a time (as here), but it's not allowed to write more than 4096 bytes by the stream APIs...
![]() |
Reporter | |
Comment 7•18 years ago
|
||
Maybe we _can_ work around this on the caller somehow... not sure...
Assignee | ||
Comment 8•18 years ago
|
||
> 1) We call requestWrite with 4096 bytes. It's in the ssl_writing_done state.
> 2) si->mThreadData->mSSLResultRemainingBytes is some small number N (8 bytes,
> or 1 byte, or 163 bytes, whatever).
> 3) It sets the state to ssl_idle and returns N
> 4) We call requestWrite with 4096 - N bytes. It's in the ssl_idle state. It
> copies 4096 - N bytes into its buffer, and sets state to ssl_pending_write.
> 5) It returns -1 and sets the error to WOULD_BLOCK.
> 6) We call requestWrite with 4096 bytes.
The previous call was (4096-N).
It got rejected with WOULDBLOCK.
But now the caller requests 4096?
Doesn't that mean the caller was able to fill up its buffer?
If the caller is able to fill up its buffer to 4096 on WOULDBLOCK, could it attempt to fill up its buffer on a short write, too?
That way it could do write requests close to 4096 most of the time?
Assignee | ||
Comment 9•18 years ago
|
||
(In reply to comment #6) > > But how would the implementation of requestWrite know, whether or not the > > caller will request to write more data? > > Because on the previous call the caller tried to write more and we didn't > consume all of it? Yes, we could immediately take the remaining bytes. This is the same amount of bytes the caller will request to write next. But because we don't know whether more bytes will follow, we must send it out immediately. Either we take the bytes immediately, or we wait for the next requestWrite that will occur with the same number of bytes - it will result in the same call to the underlying socket - because we don't know whether more bytes will follow. (Unless we'd add a delay + timer)
![]() |
Reporter | |
Comment 10•18 years ago
|
||
> Doesn't that mean the caller was able to fill up its buffer? Yes, but it doesn't know that... > could it attempt to fill up its buffer on a short write, too? Not sure. Trying to do that, in fact. Part of the thing to keep in mind is that there are at least 4 separate stream classes interacting here. And the outermost stream is told (by the HTTP code) to send 4096 bytes and no more. So the only way to "fill the buffer" would be to go out all the way to the outermost caller somehow. I'm trying to create a patch that would do that, but we'll see. The reason the WOULD_BLOCK let it fill up the buffer is that it got propagated all the way out... > Yes, we could immediately take the remaining bytes. I guess that would only help insofar as it would make the packet sequence N, 4096-N, 4096, N, ... (assuming you only grow your buffer to twice the caller's buffer size, of course).
![]() |
Reporter | |
Comment 11•18 years ago
|
||
What this patch does is to make sure that we start each substream of a multiplex input stream on a 4096-byte boundary. In effect, we align the start of the file we're uploading with the start of a 4096-byte block, and then read the file in those blocks. We don't recover from oscillations if they appear (can't, in fact), but we eliminate the oscillations that were due to the file not lining up on a 4096-byte boundary. This eliminates the oscillations I was seeing. HOWEVER.... With this patch, I see us make a number of calls to nsSSLThread::requestWrite and ssl_SecureSend with 4096 bytes as the length. nsSSLThread::requestWrite alternates between returning 4096 and -1, everything is peachy... until a point when nsSSLThread::requestWrite only consumes 4095 bytes. Presumably this happens when ssl_SecureSend returns "one less than the amount passed in", as described in bug 356470 comment 54. The problem, of course, is that after that point we start oscillating in the 4095/1 pattern originally observed in bug 356470. Could we make the PSM code ASS-ume that if all but one of the bytes it asked to send were read that actually means all the data was read, propagate _that_ answer to its callers, then make sure to resend the one byte to make this all work out? That probably means posting an event to send the remaining byte, and canceling the event if we get another write from the caller or something.... Otherwise we reintroduce bug 80092, for all intents and purposes. Of course the same code could live in NSS, not PSM, with the same effect. That's how I wish bug 80092 had been fixed to start with...
Assignee | ||
Comment 12•18 years ago
|
||
(In reply to comment #11) > With this patch, I see us make a number of calls to nsSSLThread::requestWrite > and ssl_SecureSend with 4096 bytes as the length. nsSSLThread::requestWrite > alternates between returning 4096 and -1, everything is peachy... until a point > when nsSSLThread::requestWrite only consumes 4095 bytes. Presumably this > happens when ssl_SecureSend returns "one less than the amount passed in", as > described in bug 356470 comment 54. > > The problem, of course, is that after that point we start oscillating in the > 4095/1 pattern originally observed in bug 356470. > > Could we make the PSM code ASS-ume that if all but one of the bytes it asked to > send were read that actually means all the data was read, propagate _that_ > answer to its callers, then make sure to resend the one byte to make this all > work out? Simpler idea: We could declare the SSL thread shall try harder to flush the buffer it has already received. When the SSL thread discovers that it could not write out all its data, it could immediately do an additional call with the remaining bytes. In the special case where the SSL layer returned a short write, IIUC this additional call to write one byte will succeed with a return value of 1. Would that work? I hacked a patch, Boris do you want to try it?
Assignee | ||
Comment 13•18 years ago
|
||
Assignee | ||
Comment 14•18 years ago
|
||
hmm, in that patch I might also handle the case where moreBytesWritten == 0. Will look into that tomorrow.
![]() |
Reporter | |
Comment 15•18 years ago
|
||
> When the SSL thread discovers that it could not write out all its data, it > could immediately do an additional call with the remaining bytes. Ah, hmm. Yeah, that would work. In fact, you would only need to do that if all but 1 bytes were read, if I understand the NSS setup correctly. > In the special case where the SSL layer returned a short write, IIUC this > additional call to write one byte will succeed with a return value of 1. I _think_ that's right. Nelson, can you sanity-check this idea? Kai, I can try tomorrow; let me know whether you want me to try the patch you attached or wait for the thing you're planning to look into, ok?
Comment 16•18 years ago
|
||
FYI, Here is some documentation on libSSL internals that I sent to Boris on 4/24. When ssl_SecureSend is called to send an amount N <= 16K on a non-blocking socket, I believe it will return only one of these values: N all data was encrypted and successfully written to underlying TCP socket. N-1 all data was encrypted, but not all encrypted data was accepted by the underlying TCP socket, and some remains buffered. This is a "short write". Note that if N-1 == 0, then ssl_SecureSend will return -1 with PR_WOULD_BLOCK_ERROR. -1 with error PR_WOULD_BLOCK_ERROR (EWOULDBLOCK), no data was sent. -1 with some other fatal error. In this case, ssl_SecureSend must not be called again on this same socket. In addition, when ssl_SecureSend is called to send amounts of more than 16KB on a non-blocking socket, it may also return these "short write" values, which signify that some data was sent, but not all of it: m*16K (a non-zero integer multiple of 16K, less than N) (m*16K)-1 Note that if N == 4K, this function cannot return numbers such as 193 or (4K - 193).
Assignee | ||
Updated•18 years ago
|
Attachment #262736 -
Attachment is obsolete: true
Assignee | ||
Comment 17•18 years ago
|
||
Comment on attachment 262736 [details] [diff] [review] experimental SSL thread patch (In reply to comment #15) > Kai, I can try tomorrow; let me know whether you want me to try the patch you > attached or wait for the thing you're planning to look into, ok? yes, please wait for the next patch
Comment 18•18 years ago
|
||
So, the challenge/question is how nsSSLThread::requestWrite knows that the caller has no more data to write, right? A timeout has been suggested, but may be complicated. Here's another idea. The caller uses a zero-length write to tell nsSSLThread::requestWrite that it has no more data to write immediately, and that nsSSLThread::requestWrite should flush its buffer out to SSL. A zero length write need not necessarily imply EOF, and it wouldn't hurt (other than being inefficient) to do it more often than necessary.
![]() |
Reporter | |
Comment 19•18 years ago
|
||
> The caller uses a zero-length write
Can't do that. The caller that "knows" that there is nothing more to write is not calling nsSSLThread::requestWrite directly, and the APIs it's going through to do it are frozen and can't be used to do zero-length writes.
Redesigning all of Necko is a lot more complicated than anything nsSSLThread would have to do to deal with this situation, trust me.
Updated•18 years ago
|
Assignee: nobody → kengert
Flags: blocking1.9? → blocking1.9+
Assignee | ||
Updated•18 years ago
|
Priority: -- → P2
Target Milestone: --- → mozilla1.9beta1
![]() |
Reporter | |
Comment 20•18 years ago
|
||
Kai, note that we probably want this on branches too, since the NSS changes that cause problems here landed on the branch (and since we had to back out another fix we wanted on branch due to what I suspect is basically this problem)... It might be good to try to coordinate the scheduling with the branch release schedule.
Assignee | ||
Comment 21•18 years ago
|
||
My assumption was, this is not critical, because we still behave correctly. But I wonder if this could also be the cause for bug 377481, bug 368611, bug 380744, we should find out. I want to work on this next.
![]() |
Reporter | |
Comment 22•18 years ago
|
||
We may behave correctly, but we trigger a bug in a widely deployed web server... The result are unacceptable regressions from a branch update, from a user's point of view.
Assignee | ||
Comment 23•18 years ago
|
||
(In reply to comment #15) > > When the SSL thread discovers that it could not write out all its data, it > > could immediately do an additional call with the remaining bytes. > > Ah, hmm. Yeah, that would work. In fact, you would only need to do that if > all but 1 bytes were read, if I understand the NSS setup correctly. Actually, I fear it's not that simple. libSSL returns N-1 if it could not flush out all data. If SSLThread immediately calls write again on the nonblocking socket, it won't help, right? Chances are, the buffers are still full. So SSLThread would have to wait for a bit. But waiting causes us to block on all SSL sockets, so we shouldn' do that. Boris, your other idea (SSL thread could report to caller that N+1 was written) seems difficult. Assuming the caller is done with all its data, who will trigger SSLThread to flush the remaining byte? I believe we require another call to write, in order to give libSSL a chance to flush out the remaining bytes. Ideally, I'd prefer to avoid reporting numbers based on assumptions. So, time for a new idea. If the SSLThread detects a short write (exactly one byte remaining), it could: - make a note of this state internally - wake up the caller, but report back WOULDBLOCK - assume the caller will try again (sooner or later the underlying socket will become writeable again and the caller will wake up?) - we arrive at SSLThread::requestWrite and detect there is only a single byte left to be sent - try to the single byte. If for some reason we still get wouldblock, report that back (again) - finally writing the single byte succeeds, and we report back the full amount I'll try to hack a patch now.
Assignee | ||
Comment 24•18 years ago
|
||
This patch implements the idea explained in the previous comment. It applies to both 1.8 branch and trunk. I tested this with plain-smtp-over-ssl. Without the patch, I get oscillation. With patch I see a stable transfer of 4096 byte blocks. (I also tested sending the same message with an S/Mime signature, this gave me osciallation between 42 and 4054 bytes, but I believe this is the caller's fault.) Boris, would you be be able to try this patch? (Sorry for the delay)
Attachment #265722 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 25•18 years ago
|
||
I think this patch is a bit easier to review, because I changed some indendation, which is not visible in this patch.
Attachment #265723 -
Flags: review?(bzbarsky)
![]() |
Reporter | |
Comment 26•18 years ago
|
||
I'm not going to be able to test this until June 2 at best... I'm out of town until then. Do you still see the S/Mime oscillation if you apply my first patch in this bug? As for review... am I really the right person? Are there no PSM peers? :( I'm not really going to be able to review until June 2 at best either; mid-July is more likely.
Assignee | ||
Comment 27•18 years ago
|
||
(In reply to comment #26) > Do you still see the S/Mime oscillation if you apply my first patch in this > bug? On trunk, I do not get oscillation with S/Mime SMTP. Both with and without your patch. On 1.8 branch, I get oscillation with S/Mime SMTP. But your patch does not apply to 1.8 branch. Do you propose to backport your patch to 1.8 branch? > As for review... am I really the right person? Are there no PSM peers? :( There are peers, sure. I was interested in your opinion because you had worked on this already :-)
Assignee | ||
Comment 28•18 years ago
|
||
Boris, you nominated this bug for the 1.8.0 branch. However, the SSL thread is not contained on that branch, it was new in 1.8.1, Firefox 2.0. I think we can remove the nomination for 1.8.0 branch. Looking at the original problem in bug 356470, we were told, FF 1.5.x does not have the bug.
Flags: blocking1.8.0.13?
Assignee | ||
Comment 29•18 years ago
|
||
(In reply to comment #27) > But your patch does not apply to 1.8 branch. > Do you propose to backport your patch to 1.8 branch? Looks like 1.8 branch does not contain nsThreadUtils.h / nsRunnable, so probably your patch depends on new thread manager changes?
Comment 30•18 years ago
|
||
Kai, the latest FF 1.5.0.x, which also uses NSS 3.11.x, should have this bug. The original problem in bug 356470 was reported while FF 1.5.0.x was still using NSS 3.10.x (FF 1.5.0.6 and 1.5.0.7 were mentioned in the bug), and bug 356470 comment 61 seems to indicate that FF 1.5.0.10 has the HTTPS file upload bug.
Assignee | ||
Comment 31•18 years ago
|
||
(In reply to comment #30) > Kai, the latest FF 1.5.0.x, which also uses NSS 3.11.x, should have this bug. > The original problem in bug 356470 was reported while FF 1.5.0.x was still > using NSS 3.10.x (FF 1.5.0.6 and 1.5.0.7 were mentioned in the bug), and > bug 356470 comment 61 seems to indicate that FF 1.5.0.10 has the HTTPS > file upload bug. Ok, thanks for the clarification. This means we'd need a different fix for 356470 on the 1.8.0 branch, because there is no SSL thread on that branch.
Assignee | ||
Comment 32•18 years ago
|
||
re-adding the blocking 1.8.0 branch ? flag
Flags: blocking1.8.0.13?
Assignee | ||
Comment 33•18 years ago
|
||
Because my earlier results were not clear, I decided to run a larger series of tests. I tried many combinations of trunk-or-branch, with-or-without-patches, and different-tests. Here are my results: 1.8 branch. 1.8 branch 1.8 branch no patches bz+kaie patches kaie patch signed mail smtp sarttls stable stable stable imap-s sent jumping stable stable clear mail smtp starttls stable stable stable imap-s sent jumping 42+4054 stable upload jumping 165+3931 164+3932 problems? yes, xul.mfasl startup crash trunk trunk trunk trunk no patches bz+kaie patches kaie patch bz patch signed mail smtp sarttls jumping stable stable jumping imap-s sent jumping 42+4054 42+4054 jumping clear mail smtp starttls jumping stable stable jumping imap-s sent jumping stable 42+4054 jumping upload jumping stable 165+3931 jumping problems? all trunk builds fail on first attempt to copy mail to sent folder over imap-s stable means: the initial exchange might have small packages but the bulk transfer until the end uses 4096 packets jumping means: the bulk transfer uses alternating small and large packets, where the size constantly changes n+n e.g. 42+4054 means: the initial exchange might have small packages but the bulk transfer uses a stable alternation between the two given packate sizes, 42, 4054, 42, 4054, 42, ...
Assignee | ||
Comment 34•18 years ago
|
||
Here is my summary for the 1.8 branch based on my test results: The patch proposed by bzbarsky for the 1.8 branch (bug 356470 attachment 262592 [details] [diff] [review]), which I used in my tests on the 1.8 branch, seems to be problematic. With that patch applied, we seem to get corrupt .mfasl files in the profile directory. After each run the following attempt to start the application crashed. I had to delete all *.mfasl files in my profile, then starting it up worked again. When comparing the columns 1.8 branch 1.8 branch bz+kaie patches kaie patch there was no (real) difference in behaviour. I said no "real" difference, because we have one 42+4054 vs. stable difference. But this seems to be really random behaviour, caused by the caller. Sometimes I get stable, sometimes I get 42+4054. I conclude (bug 356470 attachment 262592 [details] [diff] [review]) should not be used on 1.8 branch. While my patch v3 works fine to mail+ssl on the 1.8 branch, it still does not fix the file upload osciallation. I do not know whether the 165+3931 oscillation is "good enough". (I could not reproduce any failures with the testcase from bug 356470, I suspect the server got fixed). TODO: If we require full stability on 1.8 branch for file uploads, I think someone would have to come up with a better networking / xpcom io patch for 1.8 branch.
Assignee | ||
Comment 35•18 years ago
|
||
Here is my summary for the trunk based on my test results: Using only my patch v3, we see an improved situation (identical to the 1.8 branch situation): - we get stable behaviour for mail, most of the time - we sometimes get a little oscillation for mail - we get improved behaviour for file upload (but still a little bit oscillating) Using both Boris' patch from this bug (attachment 262729 [details] [diff] [review]) and my patch v3, we see an additional improvement: - file upload is now always stable - (but we still get a little bit of oscillation with mail) I propose we take both patches for trunk.
Assignee | ||
Comment 36•18 years ago
|
||
Comment on attachment 262729 [details] [diff] [review] xpcom+netwerk patch v1, trunk only Renaming Boris attachment from original: "_still_ doesn't work... but now the problem is definitely in PSM/NSS land" to "xpcom+netwerk patch v1, trunk only"
Attachment #262729 -
Attachment description: _still_ doesn't work... but now the problem is definitely in PSM/NSS land → xpcom+netwerk patch v1, trunk only
Assignee | ||
Comment 37•18 years ago
|
||
Comment on attachment 265722 [details] [diff] [review] ssl-thread patch v3 (trunk and 1.8 branch) [THIS ONE checked in] renaming "Patch v3" to "ssl-thread patch v3 (trunk and 1.8 branch)"
Attachment #265722 -
Attachment description: Patch v3 → ssl-thread patch v3 (trunk and 1.8 branch)
Assignee | ||
Updated•18 years ago
|
Attachment #265723 -
Attachment description: whitespace ignore version of patch v3 → whitespace ignore version of ssl-thread patch v3 (trunk and 1.8 branch)
![]() |
Reporter | |
Comment 38•18 years ago
|
||
> Do you propose to backport your patch to 1.8 branch? Possibly. Depends on what the HTTP oscillation looks like once the SSL changes happen. And yeah, I'd need to look up what all those APIs look like there and whatnot. :( > I was interested in your opinion because you had worked on this already :-) If it fixes the "clamp oscillations to 1/4095" issue, I'm pretty happy, I think.... ;) I could sr, I guess, but I'd want someone who knows this code to r=. > The patch proposed by bzbarsky for the 1.8 branch (bug 356470 attachment > 262592 [details]), which I used in my tests on the 1.8 branch, seems to be > problematic. Oh, that patch is just wrong. I figured that out while working on the patch in this bug, but just forgot to mark it obsolete. Sorry about that. We'll just need to port my patch from this bug to branch, if possible and wanted.
![]() |
Reporter | |
Comment 39•18 years ago
|
||
So my point is that someone familiar with this code should review this before I do, esp since I'm not sure when I'll be able to do it. :( It might well not be until mid-July at this point; my time for working on Mozilla things basically ran out about a week ago.
Assignee | ||
Comment 40•18 years ago
|
||
Good news. I might have said earlier that I'm unable to reproduce. Yes, I'm still unable to reproduce on Linux. But I am finally able to reproduce the original bug on Windows, using the testcase from bug 356470 at https://staging.sr.admission.net/joetemp/file_upload.html My first test was on the 1.8 branch, Firefox 2, Windows. It seems my Patch v3 is sufficient and makes the upload succeed. I'm currently building Firefox trunk on Windows and will test that, too.
Assignee | ||
Updated•18 years ago
|
Attachment #265722 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•18 years ago
|
Attachment #265723 -
Flags: review?(bzbarsky) → review?(rrelyea)
Assignee | ||
Comment 41•18 years ago
|
||
Windows trunk: upload fails. Windows trunk with Patch v3: upload works
Updated•18 years ago
|
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Comment 42•18 years ago
|
||
Comment on attachment 265723 [details] [diff] [review] whitespace ignore version of ssl-thread patch v3 (trunk and 1.8 branch) please check in the white space version, however.
Attachment #265723 -
Flags: review?(rrelyea) → review+
Assignee | ||
Comment 43•18 years ago
|
||
I checked in patch v3. Please go ahead and test tomorrow's nightly builds against any broken servers. We might also check this patch into the 1.8 branch for FF/TB 2.0.x after some more baking. (Unfortunately, this patch is not appropriate for the 1.8.0 branch (FF/TB 1.5.x) because it does not have a SSL thread) The oscillation is not completely gone. But I believe that any further optimization needs to happen at the levels above PSM (necko, mailnews, etc.). (and any optimization attempts on the 1.8.0 branch for FF/TB 1.5.x, too)
Assignee | ||
Comment 44•18 years ago
|
||
Comment on attachment 265723 [details] [diff] [review] whitespace ignore version of ssl-thread patch v3 (trunk and 1.8 branch) Requesting approval for 1.8.1.5 for FF/TB 2.0.0.x But proposing a couple of days additional trunk baking.
Attachment #265723 -
Flags: approval1.8.1.5?
Comment 46•18 years ago
|
||
Not a blocker, but a fix would be nice so we'll look at the approval request. Given the checkin is this now "FIXED" on trunk? Feedback from that is one thing we'll be looking for before approving the trunk patch.
Assignee | ||
Comment 47•18 years ago
|
||
This is now "MUCH IMPROVED" on trunk. It is "FIXED for me" when I tested the testcase from bug 356470. I wouldn't call it completely fixed, as someone might want to improve Necko to completely eliminate all kinds of oscillation.
Assignee | ||
Updated•18 years ago
|
Attachment #265722 -
Attachment description: ssl-thread patch v3 (trunk and 1.8 branch) → ssl-thread patch v3 (trunk and 1.8 branch) [THIS ONE checked in to trunk]
Comment 48•18 years ago
|
||
Let's mark this one FIXED and file another bug if there are additional imporovements needed.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
![]() |
Reporter | |
Updated•18 years ago
|
Attachment #262729 -
Attachment is obsolete: true
Comment 50•17 years ago
|
||
Comment on attachment 265723 [details] [diff] [review] whitespace ignore version of ssl-thread patch v3 (trunk and 1.8 branch) approved for 1.8.1.5, a=dveditz for release-drivers
Attachment #265723 -
Flags: approval1.8.1.5? → approval1.8.1.5+
Assignee | ||
Updated•17 years ago
|
Attachment #265722 -
Attachment description: ssl-thread patch v3 (trunk and 1.8 branch) [THIS ONE checked in to trunk] → ssl-thread patch v3 (trunk and 1.8 branch) [THIS ONE checked in]
You need to log in
before you can comment on or make changes to this bug.
Description
•