On Wed, 17 May 2006 16:31:56 +0200, Andrea Arcangeli <andrea@cpushare.com> wrote:

I noticed processEnded is called by threads. This generates a subtle race condition for most code (or at least for my code). The fact processEnded is called from threads isn't documented anywhere and so I prefer to be safe than sorry since I assume most code is written thinking processEnded will run in the usual serialized context and not in a parallel racy thread.

No it isn't. The bug you imagine here is non-existent. There is nothing to be fixed. Jean-Paul

Andrea don't understand the purpose of that code (he's effectively reading it backwards) and disabling it will make the situation with processes and threads worse, not better. Please don't apply this patch (or any of his patches) to your own copies of Twisted.

I didn't want to answer but I think I should to confirm that for this single patch (2/4 posted to python-twisted) they're right: I misread the code because defertothread(callback) means defer the callback to the thread, but callfromthred _doesn't_ mean to call the callback from the thread. callfromthread means to call the callback from the main event loop instead, great API naming I would say. But for all other patches I posted, they should be applied to everybody's own twisted copy. Including the "no it isn't", since the bug exists and needs fixing or I get exceptions. If trac would have posted me updates via email I wouldn't have been forced to resubmit here. I got zero feedback and I've better things to do than to poll trac (I even tried to poll track but I couldn't find my tickets anymore, probably because you closed them and I got no notification by email). Perhaps I didn't include myself in some CC list or whatever, but in all bugtracking software I used in my life so far, I never needed to add myself to a CC list if I was the very submitter of the report, perhaps trac is different I don't know, apologies for that if that's the case. Overall using twisted for my project was a mistake. One major core design problem it has is that it can't pass POLLERR|POLLHUP|POLLNVAL to the poll syscall unless POLLIN/POLLOUT are set too. This means that if I throttle the I/O I can't notice anymore when a disconnect happens. But the severe core design issues it has would be fixable and they are a minor problem compared to the behaviour of some elements of the community that exists around it. I don't need to enter the details myself, if doc/fun/Twisted.Quotes isn't enough, quoting emails like these is more than enough: http://twistedmatrix.com/pipermail/twisted-python/2006-May/013137.html http://www.mail-archive.com/twisted-web%40twistedmatrix.com/msg00352.html (sadly there are many more) The only bit for which I prefer to enter into the details (just in case somebody didn't notice) is that the guy puts me words in my mouth that I never said and that I never thought either. Specifically I never said unittest are a waste of time, I only said that I don't have time to write unittests and that lack of unittests should not prevent valid bugfixes to be included and I think this is a very reasonable development approach. We can argue about features or huge changes, but I think valid bugfixes should not require unittests. If somebody sends me a huge amount of unittests I will apply them immediately. The more unittests the better! Exactly the opposite of "unittests are a waste of time" that the guy claimed that I have said. I even written "unittests are welcome" it in the CPUShare-Twisted homepage, the last change in that page happened 2006-02-16, so quite a few months ago. http://www.cpushare.com/twisted In the CPUShare-Twisted fork fixes and new features will be applied without formalities. Notably, unit-tests are welcome, but they're certainly not mandatory for inclusion of new features and bugfixes. If you've objectives to reach quickly and you don't want having to maintain your own version of Twisted, you may find this project useful to you.

On Thu, 18 May 2006 21:25:07 +0200, Andrea Arcangeli <andrea@cpushare.com> wrote:

...

Andrea don't understand the purpose of that code (he's effectively reading it backwards) and disabling it will make the situation with processes and threads worse, not better. Please don't apply this patch (or any of his patches) to your own copies of Twisted.

I didn't want to answer but I think I should to confirm that for this single patch (2/4 posted to python-twisted) they're right: I misread the code because defertothread(callback) means defer the callback to the thread, but callfromthred _doesn't_ mean to call the callback from the thread. callfromthread means to call the callback from the main event loop instead, great API naming I would say.

I understand that English isn't your first language, and I don't blame you for that, but you can find the documentation for these APIs here: http://twistedmatrix.com/documents/current/api/twisted.internet.interfaces.I... and I think the descriptions are easy enough to understand. The API logic is this: If you have a function and you are running in the reactor, but you want to CALL that function IN a THREAD, you use callInThread. If you are running in a thread already, and you want to CALL a function that uses Twisted APIs (such as the reactor) FROM that THREAD, you use callFromThread. Perhaps "invokeArgumentInThreadpool" or "dispatchCallableToReactorForThreadSafety" would have been better names, but I already wear out my keyboards fast enough.

But for all other patches I posted, they should be applied to everybody's own twisted copy. Including the "no it isn't", since the bug exists and needs fixing or I get exceptions.

Since you've already demonstrated that you don't bother to read the documentation, write tests, or even understand the difficulties that may or may not exist before you submit a patch and apply it to your branch, I don't doubt that tracebacks happen, but I doubt that they're the problem you have identified. The reason we require unit tests is the same reason scientists require *reproducible* experiments. If you don't submit a test that can easily produce the traceback you saw, how else would we know that some local misconfiguration on your system wasn't causing it.

If trac would have posted me updates via email ...

o/` If I only had a brain o/` da dee da dee da da dum o/` Trac isn't perfect, but despite some minor complaints everyone else is managing to use it. Like Twisted, Trac is written in Python, and if you want it to send you email, you can submit plugins and suggest that we use them. Also, you can learn how to click on a bookmark, or even update an RSS feed. Here's a single convenient page you can poll for all your tickets - including those that have been closed - and it does have an "RSS" button at the bottom so you can monitor changes: http://twistedmatrix.com/trac/query?reporter=andrea

Overall using twisted for my project was a mistake.

And yet, and yet, and yet...

One major core design problem it has is that it can't pass POLLERR|POLLHUP|POLLNVAL to the poll syscall unless POLLIN/POLLOUT are set too.

This is definitely a bug. I wouldn't term it a "major core design problem", since I think it can be affected even without changing any external APIs. Oddly enough, you submitted a bug in the tracker about it, which could have been found either with trac's search field or the above query. In fact it's also the first hit for "POLLHUP POLLNVAL twisted" on google. However, since both Trac and Google are far too much work for busy, important men like yourself, I've included a link here: http://twistedmatrix.com/trac/ticket/1662 A response has been posted within the last month, with a comment indicating that it will be fixed.

This means that if I throttle the I/O I can't notice anymore when a disconnect happens.

The last paragraph of the ticket's description begins: "Currently I don't strictly need this" I doubt that we'd necessarily do anything that was important to you in particular, but *even you* have said that this isn't really that important. It is hypocrisy of the worst order to bring it up in this discussion as a critical, severe problem that indicates the failure of the whole community.

But the severe core design issues it has would be fixable and they are a minor problem compared to the behaviour of some elements of the community that exists around it. I don't need to enter the details myself, if doc/fun/Twisted.Quotes isn't enough, quoting emails like these is more than enough:

"I know you are but what am I!?!?" You're the one who won't follow simple rules of politeness like "use the tracker" or "write unit tests if you require your patches be applied". You're the one who throws around baseless insults and lies about other peoples' code. Your factual errors about Nevow during the period when you were abandoning it were too numerous to even taxonomize -- when have I ever said anything nasty about CPUShare? Even considering these abuses, we still haven't rejected your access to our resources, including our mailing list and our bugtracker. In fact, as evidenced above, effort and thought is going into fixing issues that you've reported! And yet, because somebody once told a fart joke on an IRC channel, you *still* have the gall to say that our "behavior", as a whole community, is too difficult for anyone to deal with. Since explicit is better than implicit, why don't I just say this right out: you are a humorless, self-important, obnoxious jerk. You need to seriously consider your own behavior before you start insulting others.

Specifically I never said unittest are a waste of time

Ahem. From http://twistedmatrix.com/pipermail/twisted-web/2006-January/002389.html """

So I'm going to fork twisted into a private twisted-CPUShare branch for my own server use where I won't have to waste time to fix bugs """

At least as I understood it, "waste time" == "write unittests" in that context, since the whole point of this fork is that you didn't want to write tests to get bug-fixes applied. You are correct that you didn't ever write, word for word, "unit tests are a waste of time" but numerous other things you said implied it.

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Indeed. <http://twistedmatrix.com/trac/ticket/1726>! Scott Lamb wrote:

Actually, English is my first language, and I'm confused every time I see those functions. It's probably too late to change the name, but might I suggest changing the first sentence of callFromThread's docstring from:

Call a function from within another (i.e. non-reactor) thread.

to:

Cause a function to be executed asynchronously on the reactor thread.

This is the language used by SwingUtilities.invokeLater(), which I've always found comparatively clear.

http://java.sun.com/j2se/1.4.2/docs/api/javax/swing/SwingUtilities.html#invo...)

Regards, Scott -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEbQnz3A5SrXAiHQcRAnbCAKCOvvp/SDJjEoHuxCjHjPNq3aZfigCffFbA yMD3vh9kAg7Y8cLOWqrG2CI= =ngPU -----END PGP SIGNATURE-----

On Thu, May 18, 2006 at 09:25:07PM +0200, Andrea Arcangeli wrote:

http://twistedmatrix.com/pipermail/twisted-python/2006-May/013137.html http://www.mail-archive.com/twisted-web%40twistedmatrix.com/msg00352.html

(sadly there are many more)

Within a few hours here one more that can be added to the list: http://twistedmatrix.com/pipermail/twisted-python/2006-May/013158.html The guy is kind enough not to blame me for not speaking english as my first native language, but yet he's trying to teach me english by saying that unittests are a "waste of time" ---quote--- This from a man who believes that unit tests are a "waste of time". ---quote--- means the same as "I won't have to waste time to fix bugs". In my minimal english language understananding, it was clear that with "wasted time to fix bugs" I meant "the time between fixing the bug and deploying the bugfix in production (or to the userbase)". I never said, believed, thought or meant that writing an unit test is a waste of time at large, as proven by the CPUShare-Twisted homepage that explicitly says for the last 3 months "unittest are welcome". Why should they be welcome if they are "a waste of time"? But then the fact the guy puts words in my mouth that I never said in order to make me look like an idiot is after all a minor thing compared to the rest he says. But let's ingore the usual politcally correct behaviour I receive from some element of this community and let's stick to technical facts: 1) callFromThread should be renamed callFromReactor (no need to make up long names when it actually means callFromReactor) 2) verifying a bugfix correctness doesn't require unittests in most cases, anybody with a basic CS knowledge perfectly knows that no matter how much testing you do you can never be guaranteed that the code is correct by just testing it. Infact if the test is buggy too, it may generate false positives. The same way you _have_ verify any unit-test without using yet another unittest (or you enter an infinite recursion), you can also verify that the bugfix is correct in the first place. The idea that without an unittest isn't possible to verify the correctness of a bugfix is totally wrong. An unit-test is sure useful, but it shouldn't be mandatory for quick bugfixes. And the time that it would take to write a new unittest for a new bug, before deploying the fix in production, is definitely "wasted". 3) when I posted the ticket (again I received no answer with that one because of the trac hidden feature) about POLLHUP/ERR/NVAL core reactor troubles, I thought it wasn't strictly needed. But that was a few weeks ago if I remember well. In the last few weeks things changed and now it looks strictly needed and worst of all to be fully reliable I would need it in the client too, and on the client I still allow my users to use the standard twisted from twistedmatrix.com. I will probably have to workaround it with a timer but it's still not a desiderable solution since I will have to rearm the poll syscall for a little way in unblocking mode which is theoretically unsafe. 4) his belief that the core reactor limitation of point 3 "can be affected even without changing any external APIs" (btw, I guess with his native english he actually means "fixed" and not "affected") implies the external API won't change but I can guarantee you that the semantics will change in a not-backwards-compatible way. I think the users should know if you're going to make not-backwards compatible semantical changes in the core reactor without changing the external API (i.e. transport.pauseProducing and friends) so they can check their apps won't break. 5) about the technical things that I said about nevow, axiom and epsilon previously, I'm sorry if the divmod guys didn't like it, but if they're not true, then they should not get angry and insult me, but they should post benchmarks that invalidate my claims. They are very welcome to start to invalidate my bold claim that for rendering web2+cheetah is an order of magnitude faster (and in turn more scalable) than web+nevow. 6) about the performance of web2 vs apache/cherrypy (note: this is different from point 5, this is _not_ web2+cheetah vs web+nevow), there is no reason to worry about the channel of web2. No reason to change the API. I already told foom and drier privately. The benchmark I've seen posted to the list had a tiny size per page. Unfortunately the majority of the cpu time is spent in the twisted core (i.e. basically the reactor). I measured it. There's little or nothing you can do inside web2 to speedup the per-connection overhead. Infact if you increase the size of the page, the connections per second remain the same, only the bandwidth increases. So effectively web2 has no immediate performance issues because most pages are larger than a dozen bytes. When you do the test on a normal page size, things are ok. Most of the overhead happens in the handling of the socket outside web2, and it's very similar for twisted.web and it's a fixed cost outside web2. Again I will be totally happy to be proven wrong and see that the divmod folks can make web2 go much faster with a tiny page size with a change of API inside twisted/web2 directory and without touching the reactor code.

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

And the time that it would take to write a new unittest for a new bug, before deploying the fix in production, is definitely "wasted".

I'm not a Twisted dev, but I do manage a couple open-source projects, and would like to point out: Participation in the open-source development process requires good communication skills. A unit test is an excellent way of communicating to the main developers that you've thought the problem through clearly and showed that your solution is a worthy one. Open-source developers are busy folk, so anything you can do to take their lives easier (even if you think their rules are foolish) will speed up the inclusion of your patch in the long run. Remember that the majority of Twisted developers are *volunteers*. I highly doubt that the time it would take for you to write a unit test would significantly impact *anyone* else who is using Twisted in production. - - Colin -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2.2 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFEbQyYXZxSS+Knk6ARAjB4AJ9VGy7lAb2v7qCnYz+JA4TNkXsDjACfX8pl FRmxo5loTbl7gNmP788vKTk= =bBaD -----END PGP SIGNATURE-----

On Fri, May 19, 2006 at 10:41:35AM +1000, Justin Warren wrote:

break existing code, ie: regression testing. I'm personally a big fan of this feature of test suites.

I'm a big fun of test suites as well. I only disagree with wasting time delaying the integration of a valid bugfix just because the unit test doesn't exist yet. I absolutely never said unit tests are a waste of time. For the last months I've always said that "unit tests" are _welcome_ at the top of the cpushare-twisted webpage.

Incidentally, those with slightly more CS knowledge know that it is possible, though by no means easy, to build a system that is provably correct. An investigation of the Z specification language may prove enlightening. I think you mean that a test suite proves that the system passes all the tests, not that it is bug free.

What I mean is that a test suite cannot prove the code is bug free. Nor that any bugfix is correct. If nothing else because the test suite may be buggy too. This is obvious. Clearly a test suite is welcome and can only help, but its mandatory requirement for any change to the code sounds way excessive. I perfectly know about formal demonstrations being possible too (I spoke about those matters for a long time last year in a completely different context) but they're not unit-tests (certainly not the ones you see in the twisted reposistory), so I didn't mention this to avoid further confusion. I doubt it's feasible to demonstrate Twisted bug free formally (to back my guess I remind you Alan Cox quote saying twisted is a 6m unauditable weirdness, I guess he was partly joking though). http://article.gmane.org/gmane.linux.kernel/327172 My current worries are the troubles with poll, I worry about the lack of epoll, I worry about scaling in SMP with one thread per cpu. Those are the things that should be discussed instead of receiving emails from people about lack of unit tests for fixes that can be trivially verified by reading the code.

make it so. If you have benchmarks that demonstrate your claim, and importantly, the method used to generate them, then by all means share them with others. If your claims are true, this information will help in fixing the problem. Fixing these bugs is what we all want, after all.

About the benchmarks to make an example I posted some benchmarks here: http://twistedmatrix.com/pipermail/twisted-web/2006-January/002425.html I used the klive homepage for it. You can reproduce yourself downloading it (all GPL): http://klive.cpushare.com/downloads/ Older versions uses web+nevow, newer uses web2+Cheetah (I don't remember exactly the time of the switch but it's easy to find with some diff). However here the kind of the answers I got: http://twistedmatrix.com/pipermail/twisted-web/2006-January/002428.html So I didn't post more benchmarks, nor I tried to produce an official benchmark that is easier to run than to install klive locally. Also note, quite a lot of the time of klive is spent in the database. So it's one of the worst possible benchmarks for web1+nevow vs web2+cheetah since only little time is spent for the rendering. I measured much higher html delivery speedups in other pages that weren't asking the db such cpu intensive queries. If there is interest I can produce an official benchmark (that sounds much more useful than unit-tests for every bugfix). However I guess we'll have to follow the above advice and do it on the cpushare-twisted list. Also note, if you've a better place than cpushare-twisted for things that may not be welcome here, that's fine with me. I made up this cpushare-twisted things to fit my needs, only because I didn't find any other alternate place.

On Fri, 2006-05-19 at 05:04 +0200, Andrea Arcangeli wrote:

On Fri, May 19, 2006 at 10:41:35AM +1000, Justin Warren wrote:

...

break existing code, ie: regression testing. I'm personally a big fan of this feature of test suites.

I'm a big fun of test suites as well. I only disagree with wasting time delaying the integration of a valid bugfix just because the unit test doesn't exist yet. I absolutely never said unit tests are a waste of time. For the last months I've always said that "unit tests" are _welcome_ at the top of the cpushare-twisted webpage.

Righto. How do you know it's a valid bugfix without a test? Personally, I know that I make mistakes from time to time, so I think it's handy to have something that will independantly verify if I've made one or not. Many a time I've thought I knew what was broken only to find out it was something completely different. The twisted folks appear to disagree with you about the necessity of writing a unit test as part of the bugfix integration process. Since it's their code, they get to make the rules.

...

Incidentally, those with slightly more CS knowledge know that it is possible, though by no means easy, to build a system that is provably correct. An investigation of the Z specification language may prove enlightening. I think you mean that a test suite proves that the system passes all the tests, not that it is bug free.

What I mean is that a test suite cannot prove the code is bug free. Nor that any bugfix is correct. If nothing else because the test suite may be buggy too. This is obvious.

Obvious to you and me, perhaps, but there may be others who read this to whom it isn't so obvious. Hopefully they find this thread helpful.

Clearly a test suite is welcome and can only help, but its mandatory requirement for any change to the code sounds way excessive.

I guess we'll just have to agree to disagree on this point. I support the mandatory unit test requirement.

I perfectly know about formal demonstrations being possible too (I spoke about those matters for a long time last year in a completely different context) but they're not unit-tests (certainly not the ones you see in the twisted reposistory), so I didn't mention this to avoid further confusion. I doubt it's feasible to demonstrate Twisted bug free formally (to back my guess I remind you Alan Cox quote saying twisted is a 6m unauditable weirdness, I guess he was partly joking though).

My apologies for starting a thread derail. I was simply responding to your assertion that nothing can be proven to be bug free. Rather petty of me in hindsight.

My current worries are the troubles with poll, I worry about the lack of epoll, I worry about scaling in SMP with one thread per cpu. Those are the things that should be discussed instead of receiving emails from people about lack of unit tests for fixes that can be trivially verified by reading the code.

We disagree on the 'trivially verified by reading code' part. It is my opinion that only trivially simple code can be correctly verified simply by reading it, particularly by so fallible a human as myself. Perhaps your code reading skills are far superior to my own. Unit tests help me to understand other's code. I have enough trouble understanding my own code six months after writing it. As you suggest, perhaps now is the time to move on to discussing other issues? [snip benchmarking bits] Interesting reading. My apologies to everyone else on the list for contributing to the... clutter. -- Justin Warren <daedalus@eigenmagic.com>

On Fri, May 19, 2006 at 03:30:31PM +1000, Jonathan Lange wrote: [...]

provide strong evidence for its absence. When fixing code, there must be some check that the fix is correct. The check generally takes one of four forms: informal induction from the code; manual

I think often a combination of checks are used: in particular, reading the code and seeing that the fix "makes sense" in addition to an empiricial demonstration of absence (exercising the fixed bug manually or by automated test) is very common. A fix is rarely accepted if it doesn't "make sense", even if there's an automated test case that suggests it works. [...]

If you wish to argue against our requirement for unit tests, then you must persuade us either that the evidence they provide is not strong enough to provide the listed benefits, or that the work in writing unit tests is significantly greater than the work of not writing unit tests.

I think the problem may be different perspectives. For Andrea, he's apparently already manually verified that his patches solve the problems he's seen, so that's good enough. Ignoring for a moment the issue of what it takes to get it merged into official Twisted, further effort is wasted. For people maintaining Twisted, a once-off manual verification isn't enough, for the reasons you give. So for Andrea's fixes to make the leap from "good enough for him" to "good enough to be merged into Twisted", they need more work: they need automated tests. -Andrew.

On Fri, May 19, 2006 at 11:47:54AM +1000, Andrew Bennetts wrote:

not have time to write the tests, but *we* don't have the time to deal with new code without tests, even if it is apparently more correct.

This is the bit I definitely disagree with.

If you cannot follow the coding standards for a project, then don't be offended if that project doesn't apply your patches.

I'm certainly not offended by it, but then I can't agree with a coding standard that I think is wrong. The thing I definitely agree with, is that having the unit test is certainly better than not having it, infact I'm very grateful to people like you that write unit tests, you're doing a great job.