Changes between Initial Version and Version 1 of DocumentationAnalysis/Threads/AndrewBennetts


Ignore:
Timestamp:
02/24/2006 09:58:47 PM (8 years ago)
Author:
trac
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • DocumentationAnalysis/Threads/AndrewBennetts

    v1 v1  
     1This is a '''template''' for experts in Twisted to use when reviewing documents as part of the DocumentationAnalysis project. See wiki:DocumentationAnalysis/DocumentList for further instructions about what to do with this template. 
     2 
     3Things in ''italics'' get replaced with your own text or removed. 
     4 
     5''How to review: as you go to the document, think very careful about the '''intended user''' (the template asks you to describe the intended user, and eventually this information will go into the document itself in some form). Think of the type of person you would love to hand this document to. Think of the type of questions on IRC that you'd love to be able to say "go to http://blahblahblah and see section 3".'' 
     6 
     7''What do these users know already? What do they think they know and are likely to actually be confused about? What don't they know that you want this document to teach them?'' 
     8 
     9''This is the purpose of the document. Any bit of text you read and any example in the document should be able to be explained in terms of either clarifying the user's understanding or teaching them something relevant and new. (Anything that's there and not doing one of these two things is presumably serving some other purpose of the author's and you might want to consider whether this purpose is worthy of its own document. Otherwise it should be removed.)'' 
     10 
     11''Recall that the intended user of this review itself is a documentation writer, and your role as a reviewer is to make their job easier. Therefore address problems that it is a writer's job to fix (mistakes, missing information, irrelevant digressions) and not problems that an improved document can't help with (say, the lack of common sense, imagination and intelligence in people you have cause to interact with on a day-to-day basis).'' 
     12 
     13= Review details = 
     14 
     15 * Link to document: [http://twistedmatrix.com/projects/core/documentation/howto/threading.html Using Threads in Twisted] 
     16 * Reviewer's name: Andrew Bennetts 
     17 * Review date: 14 January 2006 
     18 
     19= Document expectations = 
     20 
     21== Intended user == 
     22 
     23Before someone begins using this document, I'd really really like it if they knew: 
     24 * how to use threads in Python -- they should be familiar with the ''threading'' module, and in general be aware of the need for care with inter-thread interactions because of race conditions etc. 
     25 * the basics of working with the reactor -- starting and stopping it, what it is and what it's for. 
     26 * what a "blocking" function is, and why it's undesirable to run them in the event loop thread 
     27 * I'd also really like it if they knew that threads aren't needed as often as they might think ;) 
     28 
     29== Outcomes == 
     30 
     31Once someone has read this document they should understand: 
     32  * The methods of `IReactorThreads`: 
     33    * how and when to use `reactor.callInThread` 
     34      * including that `callInThread` is not suitable for very long-running functions, because the reactor's threadpool may become filled with such requests and thus reach a state where it will never run functions passed to it. 
     35    * how and when to use `reactor.callFromThread` 
     36    * how and when to use `reactor.suggestThreadPoolSize` 
     37  * That Twisted is not in general thread-safe, i.e. all Twisted code expects that it is running in the reactor's thread, so if other threads need to interact with Twisted code, they should use `reactor.callFromThread` to safely arrange for code to be run as part of the reactor thread. 
     38  * Perhaps how to use `twisted.python.threadpool` to create your own thread pools if you don't want clog the reactor's threadpool. 
     39  * That starting a plain Python thread is perfectly legal in Twisted, and can be a useful thing to do when a threadpool of worker threads is not appropriate. 
     40 
     41 
     42= Document review = 
     43 
     44== Coverage of subject matter == 
     45 
     46This document seems to cover the following subject matter at least acceptably well: 
     47 * that Twisted generally is not thread-safe 
     48 * when and how to use `callFromThread` 
     49 * when and how to use `callInThread`. 
     50 * `twisted.internet.threads.callMultipleInThread` -- although I don't really see the point of this utility function (running code sequentially is what functions do already). 
     51 
     52This document seems to be attempting to cover the following subject matter, but its coverage is flawed: 
     53 * `callFromThread` is introduced '''before''' `callInThread` -- I wonder if the reader will be wondering "yes, but where does this other thread come from?"  A possibly related problem is that the example code for `callFromThread` is not a self-contained, executable example, it's just a skeleton (which is probably worthwhile for understanding, but I think concrete code would also help). 
     54 * "Running code in threads"'s example is '''almost''' a complete executable program, but is missing the `reactor.run()` 
     55 * "Running code in threads" intentionally shows questionable code (using `callInThread` for `time.sleep`) for the sake of having a simple example, but either a better motivating example should be found, or it should further emphasis that this is the wrong way to do this particular task (e.g. with a docstring saying "a bad way to do `reactor.callLater(2, log.msg, x)`").  To be fair, the name "`aSillyBlockingMethod`" does try to emphasise this, but I don't think it's a clear enough signal. 
     56 * `deferToThread` is mentioned, but extremely briefly, with another stub example that can't be executed, and should probably refer to the Deferred documention as well. 
     57 
     58This document ought to be covering the following subject matter but is not: 
     59 * the reactor threadpool is being used, but the limitations of this are not mentioned 
     60 
     61 * ''the reason why Y is a better design than Z in this case'' 
     62 * ''Foo useful library function'' 
     63 * ''the X class of problems for which this approach needs to be modified'' 
     64 
     65'''This document is recommending the following things that it shouldn't be recommending:''' 
     66 
     67The introduction flatly contradicts the source code: 
     68 
     69{{{ 
     70Before you start using threads, make sure you do at the start of your program: 
     71 
     72from twisted.python import threadable 
     73threadable.init() 
     74}}} 
     75 
     76vs. 
     77 
     78{{{ 
     79def init(with_threads=1): 
     80    """Initialize threading. 
     81 
     82    Don't bother calling this.  If it needs to happen, it will happen. 
     83    """ 
     84}}} 
     85 
     86This apparently deprecated code is repeated in the examples throughout the document. 
     87 
     88 
     89 * ''X out-of-date library function'' 
     90 * ''Y sequence of calls which have been superceded by Z library'' 
     91 
     92This document could be supplemented by links to these existing pre-requisitie ("you should read this first") documents: 
     93''list here'' 
     94 
     95This document could be supplemented by links to these existing follow-up ("now that you know X you can try Y") documents: 
     96''list here'' 
     97 
     98This document could be supplemented by as-yet non-existant pre-requisite ("you should read this first") documents on: 
     99''list here'' 
     100 
     101This document could be supplemented by links to these as-yet non-existant follow-up ("now that you know X you can try Y") documents: 
     102''list here'' 
     103 
     104== Style == 
     105The following changes to the style of the document would make it easier to read: 
     106 
     107 * ''shorter paragraphs in section 4'' 
     108 * ''less hyperbole about the Twisted Way in the introduction'' 
     109 
     110== Overall summary == 
     111 
     112This document is: ''excellent / good, with some minor improvements to recommend / reasonable, with some pretty obvious improvements to make / better than having no documentation, but with some glaringly obvious improvements to make / so bad that it would be better to have no documentation on this''