Ben,
This sounds a very strange question, but might have these patches change the
semantics of GetNewMsgOutputStream
as in
``` rv = m_msgStore->GetNewMsgOutputStream(m_folder, getter_AddRefs(newHdr),
getter_AddRefs(m_outFileStream));
```
ever so slightly?
You know I am working on the BUFFERED output of messages and
I work on the m_outFileStream returned by GetNewMsgOutputStream(...),
first enabling buffering of m_outFileStream
and then making sure we get to the EOF position since the buffering operation turns out to
rewind the stream to its beginning (!).
I am removing the reusable flag feature from stream handling because
- as it turned out, it is not so much of a performance win in modern hardware/software (I believe I posted the performance comparison result requested before.) The result is the win due to buffering outweighs any negative impact caused by NOT using reusable feature.
The buffering makes the copying of 1000 messages from one buffer to the other happen in less than 5 seconds under Windows while the old code
takes 90 seconds or more. (Based on real messages from gdb-mailing list, etc.)
- the reusable feature makes file descriptor handling (including caching necessitated by reusability) too complex and hairy (in the meaning of MIT hackers of the old days),
so I removed it.
So the handling of downloading a pop3 message is
IncorporateMessage begins
GetNewMsgOutputStream is called to obtain a output stream.
Buffering is enabled on that stream.
Seek to the end of the stream is performed.
Write downloaded message to that stream, <--- not sure if this is correct or not these days.
and THEN ALWAYS close the stream.
End Incorporate Message.
Problem I found in the last couple of days is that this scenario works for the first message, but
for the second message, it fails.
Even the simple test of
```
mach xpcshell-test --verbose comm/mailnews/local/test/unit/test_pop3Download.js
```
I can download the first message OK. So I am puzzled.
In the second run for the second message,
the seek fails. (The error is that the underlying buffer stream has already been CLOSED?! So somehow file stream that should have been associated with it is gone.)
The write also fails in the end (I think when the buffered output is written and the reason is the same. Underlying file stream has been closed.)
Any idea?
I know this bugzilla is mainly for INPUT, but since there is a mention
Bug 1733849 - Correct the byte counting in MailTestUtils.loadMessageToString(). r=mkmelin
so there is a remote chance something in the output function might have changed slightly. (Actually that particular patch does not
look responsible.)
You may be doing other works related to message store (including output phase) so you may have a hunch here.
NB.: I am not ruling out a subtle copy & paste error, but
the new problems I noticed only in the last couple of days (suddenly I got so many errors), seem to be related to this
and other patches possibly.
Oh, I should mention, that I have not updated my local source tree to focus on fixed problems for a few weeks.
I did not want to change the tree so often so that I can focus on known bugs.
Another reason I did not update the code is the hardware problem of my PC.
My host windows 10 pro OS failed to boot. I run linux as a guest OS inside VirtualBox that runs under windows10.
It took me almost a couple of weeks to fully recover to the point I can use VirtualBox again. OS recovery essentially wipes out registry and as it turns out VirtualBox stores the virtual disk [mapped to Windows 10 file] in registry. Agha.
Anyway, I updated the source tree three days ago or so and suddenly I noticed these new errors.
And the reason for the error is related to the seeking/writing to the stream returned by GetNewMsgOutputStream ().
Given that you seem to have changed the default download to use a temporary file (last year?) by default,
the legacy patch I have been working may have missed the subtle change it may require to cope with this temporary file.
But like I said, the latest problems I have not seen before a few days ago is related to this
strange seek fails on the second message incorporation processing and the first message handling does not fail.
(I don't believe this was the case before. But I may have to go back and check the old local logs.
But it was not that clear cut if I recall correctly...)
In the meantime, if you have any hunch, for example, like we should not close the stream returned by
GetNewMsgOutputStream() these days until the whole one pop3 download is complete or something like that
I would love to hear that.
TIA
Chiaki
Bug 1733849 Comment 18 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Ben,
This sounds a very strange question, but might have these patches change the
semantics of GetNewMsgOutputStream
as in
``` rv = m_msgStore->GetNewMsgOutputStream(m_folder, getter_AddRefs(newHdr),
getter_AddRefs(m_outFileStream));
```
ever so slightly?
You know I am working on the BUFFERED output of messages and
I work on the m_outFileStream returned by GetNewMsgOutputStream(...),
first enabling buffering of m_outFileStream
and then making sure we get to the EOF position since the buffering operation turns out to
rewind the stream to its beginning (!).
I am removing the reusable flag feature from stream handling because
- as it turned out, it is not so much of a performance win in modern hardware/software (I believe I posted the performance comparison result requested before.) The result is the win due to buffering outweighs any negative impact caused by NOT using reusable feature.
The buffering makes the copying of 1000 messages from one buffer to the other happen in less than 5 seconds under Windows while the old code
takes 90 seconds or more. (Based on real messages from gdb-mailing list, etc.)
- the reusable feature makes file descriptor handling (including caching necessitated by reusability) too complex and hairy (in the meaning of MIT hackers of the old days),
so I removed it.
So the handling of downloading a pop3 message is
IncorporateMessage begins
GetNewMsgOutputStream is called to obtain a output stream.
Buffering is enabled on that stream.
Seek to the end of the stream is performed.
Write downloaded message to that stream, <--- not sure if this is correct or not these days.
and THEN ALWAYS close the stream.
End Incorporate Message.
Problem I found in the last couple of days is that this scenario works for the first message, but
for the second message, it fails.
Even the simple test of
```
mach xpcshell-test --verbose comm/mailnews/local/test/unit/test_pop3Download.js
```
I can download the first message OK. So I am puzzled.
In the second run for the second message,
the seek fails. (The error is that the underlying buffer stream has already been CLOSED?! So somehow file stream that should have been associated with it is gone.)
The write also fails in the end (I think when the buffered output is written and the reason is the same. Underlying file stream has been closed.)
Any idea?
I know this bugzilla is mainly for INPUT, but since there is a mention
Bug 1733849 - Correct the byte counting in MailTestUtils.loadMessageToString(). r=mkmelin
so there is a remote chance something in the output function might have changed slightly. (Actually that particular patch does not
look responsible.)
You may be doing other works related to message store (including output phase) so you may have a hunch here.
NB.: I am not ruling out a subtle copy & paste error, but
the new problems I noticed only in the last couple of days (suddenly I got so many errors), seem to be related to this
and other patches possibly.
Oh, I should mention, that I have not updated my local source tree to focus on fixed problems for a few weeks.
I did not want to change the tree so often so that I can focus on known bugs.
Another reason I did not update the code is the hardware problem of my PC.
My host windows 10 pro OS failed to boot. I run linux as a guest OS inside VirtualBox that runs under windows10.
It took me almost a couple of weeks to fully recover to the point I can use VirtualBox again. OS recovery essentially wipes out registry and as it turns out VirtualBox stores the virtual disk [mapped to Windows 10 file] in registry. Agha.
Anyway, I updated the source tree three days ago or so and suddenly I noticed these new errors.
And the reason for the error is related to the seeking/writing to the stream returned by GetNewMsgOutputStream () for the second message.
The first message is handled OK.
Given that you seem to have changed the default download to use a temporary file (last year?) by default,
the legacy patch I have been working may have missed the subtle change it may require to cope with this temporary file.
But like I said, the latest problems I have not seen before a few days ago is related to this
strange seek fails on the second message incorporation processing and the first message handling does not fail.
(I don't believe this was the case before. But I may have to go back and check the old local logs.
But the error I saw was not that clear cut if I recall correctly. I could be wrong. I changed to a CPU to a slightly newer one and my build time is about 1.7 times faster than before and so I can work on patches more efficiently. That is why I could focus on the issue this weekend.)
In the meantime, if you have any hunch, for example, like we should not close the stream returned by
GetNewMsgOutputStream() these days until the whole one pop3 download is complete or something like that
I would love to hear that.
TIA
Chiaki
Ben,
This sounds a very strange question, but might have these patches change the
semantics of GetNewMsgOutputStream
as in
```
rv = m_msgStore->GetNewMsgOutputStream(m_folder, getter_AddRefs(newHdr),
getter_AddRefs(m_outFileStream));
```
ever so slightly?
You know I am working on the BUFFERED output of messages and
I work on the m_outFileStream returned by GetNewMsgOutputStream(...),
first enabling buffering of m_outFileStream
and then making sure we get to the EOF position since the buffering operation turns out to
rewind the stream to its beginning (!).
I am removing the reusable flag feature from stream handling because
- as it turned out, it is not so much of a performance win in modern hardware/software (I believe I posted the performance comparison result requested before.) The result is the win due to buffering outweighs any negative impact caused by NOT using reusable feature.
The buffering makes the copying of 1000 messages from one buffer to the other happen in less than 5 seconds under Windows while the old code
takes 90 seconds or more. (Based on real messages from gdb-mailing list, etc.)
- the reusable feature makes file descriptor handling (including caching necessitated by reusability) too complex and hairy (in the meaning of MIT hackers of the old days),
so I removed it.
So the handling of downloading a pop3 message is
IncorporateMessage begins
GetNewMsgOutputStream is called to obtain a output stream.
Buffering is enabled on that stream.
Seek to the end of the stream is performed.
Write downloaded message to that stream, <--- not sure if this is correct or not these days.
and THEN ALWAYS close the stream.
End Incorporate Message.
Problem I found in the last couple of days is that this scenario works for the first message, but
for the second message, it fails.
Even the simple test of
```
mach xpcshell-test --verbose comm/mailnews/local/test/unit/test_pop3Download.js
```
I can download the first message OK. So I am puzzled.
In the second run for the second message,
the seek fails. (The error is that the underlying buffer stream has already been CLOSED?! So somehow file stream that should have been associated with it is gone.)
The write also fails in the end (I think when the buffered output is written and the reason is the same. Underlying file stream has been closed.)
Any idea?
I know this bugzilla is mainly for INPUT, but since there is a mention
Bug 1733849 - Correct the byte counting in MailTestUtils.loadMessageToString(). r=mkmelin
so there is a remote chance something in the output function might have changed slightly. (Actually that particular patch does not
look responsible.)
You may be doing other works related to message store (including output phase) so you may have a hunch here.
NB.: I am not ruling out a subtle copy & paste error, but
the new problems I noticed only in the last couple of days (suddenly I got so many errors), seem to be related to this
and other patches possibly.
Oh, I should mention, that I have not updated my local source tree to focus on fixed problems for a few weeks.
I did not want to change the tree so often so that I can focus on known bugs.
Another reason I did not update the code is the hardware problem of my PC.
My host windows 10 pro OS failed to boot. I run linux as a guest OS inside VirtualBox that runs under windows10.
It took me almost a couple of weeks to fully recover to the point I can use VirtualBox again. OS recovery essentially wipes out registry and as it turns out VirtualBox stores the virtual disk [mapped to Windows 10 file] in registry. Agha.
Anyway, I updated the source tree three days ago or so and suddenly I noticed these new errors.
And the reason for the error is related to the seeking/writing to the stream returned by GetNewMsgOutputStream () for the second message.
The first message is handled OK.
Given that you seem to have changed the default download to use a temporary file (last year?) by default,
the legacy patch I have been working may have missed the subtle change it may require to cope with this temporary file.
But like I said, the latest problems I have not seen before a few days ago is related to this
strange seek fails on the second message incorporation processing and the first message handling does not fail.
(I don't believe this was the case before. But I may have to go back and check the old local logs.
But the error I saw was not that clear cut if I recall correctly. I could be wrong. I changed to a CPU to a slightly newer one and my build time is about 1.7 times faster than before and so I can work on patches more efficiently. That is why I could focus on the issue this weekend.)
In the meantime, if you have any hunch, for example, like we should not close the stream returned by
GetNewMsgOutputStream() these days until the whole one pop3 download is complete or something like that
I would love to hear that.
TIA
Chiaki