On 01/03/2012 22:55, Filip Hanik - Dev Lists wrote: > You can't register on a selector using another thread.
Bingo. That explains it. Is that documented anywhere? I don't recall reading it but I may well have missed it. Looking at the code, it looks like HTTP would have been unaffected which is good news. Looks like this problem started with [1] which fixed some other issues with Comet but introduced this one. My bad. > On a completely separate note, this WebSocket implementation seems to > have caused a lot of duplicate code. There is a little duplicate code in the UpgradeProcessor implementations to read the actual bytes but apart from that it is all pretty much new code. There is probably a way to refactor some of that into the endpoints (since it is really endpoint specific) but I haven't looked at that. > I would have assumed the most > simple way to implement WebSockets would be on top of the existing Comet > implementation. There are a couple of problems / concerns I would have with doing that. 1. Comet is not implemented for BIO and a generic upgrade process ideally needs to support all three connectors. Further, there is very likely to be some form of generic upgrade support in Servlet 3.1 and that will obviously have to work with all three connectors. 2. It would mean retaining an HttpProcessor object, a Request object and a Response object for each connection. If you look back at the short history of the WebSocket implementation you'll see that Costin raised some scalability concerns about the initial implementation that used the HttpProcessor. The move to the deliberately very light-weight UpgradeProcessor was as a direct result of those concerns. 3. Comet doesn't currently support switching from HTTP to Comet. That could be added but it would increase complexity to the current Comet implementation. > if there is more data to be read. This would too have avoided single > byte reads from the system buffers, since Comet reads in as much as > possible, and then you can single byte read from Java heap memory. 4. When I looked at using / adapting the existing InputBuffers I hit various complications and I ended up adding more code to re-use the existing buffers than I needed to add if I didn't use them. 5. Not having a buffer between the system buffer and the reads doesn't seem to impact performance at all. I experimented with adding a BufferedInputStream to the existing implementation and there was a negligible impact on performance - positive or negative. (As an aside, the performance of the current implementation appears to be on par with other implementations based on the results of the Autobahn tests. Those tests are far from comprehensive as performance tests and don't test performance under load or scalability but they are the best indication we have at the moment.) > The only thing you would have had to do, had you used Comet, was to turn > off the filters that cause chunked-encoding, and you would have had > access to raw data directly from within. I disagree. There would have been more work required than that - handling the HTTP upgrade and getting it working with BIO for starters and the result would have been much more heavy-weight (and hence more likely to have scalability issues) than the current implementation. I'm not saying that these issues couldn't be overcome, but based on my initial experience heading in this direction a separate implementation was simpler to implement and will be easier to maintain. The little duplicate code this has introduced can probably be refactored out at some pointy in the future. > All the Upgrade classes, > everything, would have sat directly in the highest level, never touching > the low level connectors. > > Something food for thought.... :) Absolutely. I'm sure there is room for improvement in the current implementation. I'm not convinced that refactoring WebSocket to be built on top of Comet is the way to go but I wouldn't be at all surprised is there was some lower level refactoring that could be done to simplify the connectors generally, further reduce duplication and improve performance. Mark [1] http://svn.apache.org/viewvc?view=revision&revision=1063366 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
