forax at univ-mlv.fr
Thu Jan 24 15:41:52 UTC 2008
Alan Bateman a écrit :
> Rémi Forax wrote:
>> Hi all, i currently develop a small web server and I think codes
>> to selectors can be improved just by changing some small pieces of code.
>> To be crystal clear, i don't want to re-implement all selector
>> related stuffs but
>> just patch some parts of the actual code.
>> There are some allocations in JDK API that can be removed,
>> the code was badly retrofited to 1.5 and lot of field can be declared
>> Some methods/fields still 'use' raw types and doesn't take
>> advantage of autoboxing.
> You're right. Much of the code here dates back to 1.4 and we haven't
> gone back to clean-up things like this.
>> Futhermore, there is some divergence between Windows and *nix
>> code i don't understand.
>> By example, WindowsSelectorImpl and PollSelectorImpl uses a pipe to
>> implements wakeup but WindowsSelectorImpl relies on Pipe
>> and PollSelectorImpl on IOUtil.initPipe().
>> I think this code should be the same.
> Ideally we would use a socketpair for the wakeup mechanism but Windows
> doesn't support it. For this reason, Pipe is implemented as a loopback
> connection and this works okay for the wakeup mechanism too. One thing
> to mention is that PollSelectorImpl is only used now when running on
> the Linux 2.4 kernel (it's not used with the 2.6 kernel and isn't used
> on Solaris). I just mention this as someday it might become obsolete
> and we can remove it.
>> in WindowsSelectorImpl:
>> - updateSelectedKeys() use an iterator to traverse the array
>> (an ArrayList). It should use an indexed loop instead
>> to avoid Iterator allocation.
>> - field threads should be declared as an ArrayList
>> because adjustThreadsCount() supose that i can be iterate
>> using an indexed loop.
>> Furthermore, it can be generified like this:
>> private final ArrayList<Thread> threads = new ArrayList<Thread>();
> These clean-ups seem reasonable.
>> - class FDMap,
>> I don't see why FdMap need to be a class, all methods can be moved
>> as member methods of WindowsSelectorImpl without problems.
>> Futhermore, the constructor of FdMap is private (get/put/remove too)
>> so the compiler stupidly inserts accessor methods (access$000 etc.).
>> Ok, the main point, here when the code was retrofited to 1.5,
>> The new Integer() was not transformed to use Integer.valueOf()
>> to share small integers and avoid allocation if file descriptor
>> value are small.
> These are SOCKET types rather than file descriptors and unlikely to be
> in the range that Integer caches (actually it should be a Long but
> that is a story for another day).
ok, no valueOf(), i'm not an expert in Windows API.
But are you agree that class FdMap is not necessary.
>> - In class MapEntry, ski should be declared final.
>> - close(), set selectedKeys() to null doesn't allow the Set to be
>> because publicSelectedKeys contains() a reference to it.
>> in PollSelectorImpl:
>> - interruptLock should be final.
>> - close(), see WindowsSelectorImpl
>> in EpollSelectorImpl:
>> - like in poll, interruptLock should be final.
>> - hashMap fdTokey should be generified and final.
>> - close(), see WindowsSelectorImpl
>> - implRegister/implDereg
>> - They should use Integer.valueOf() instead of new Integer().
>> - IOUtil.fdVal() is used spuriously, in implRegister but not
>> in implDereg.
> These are integers so there could be some benefit (but probably very
> hard to measure).
yes, very hard to mesure until you span 1k thread with one selector each.
btw if you take a look to EPollArrayWrapper, idlSet already use boxing.
>> - EPollArrayWrapper
>> - updateList is a LinkedList, a double linked list that stores
>> Updator object,
>> I think it's more efficient to add a field next in the Updator
>> object and
>> link updator by hand in order to avoid to create LinkedList$Entry .
> Maybe but probably very hard to measure.
>> - Updataor.opcode and Updator.fd should be declared final.
>> - SelectorImpl:
>> key and selectedKeys should be LinkedHashSet instead of Set
>> because they are frequently iterated.
>> let discuss about that before I submit patchs.
> The clean-ups you suggest seem reasonable so I would suggest going
> ahead and sending a patch.
i will do that.
> I'm happy to review and work with you to get the clean-ups integrated
> (once OpenJDK/jdk7 re-opens for changes of course).
Do you have any idea when openjdk will be reopen ?
> PS: I don't know anything about your "small web server" but the simple
> server in com.sun.net.httpserver may be useful.
My small server is a research project that embeds a non-blocking parser
in a webserver and
claims to have the same performance than grizzly. I will post a blog
entry about it
when all benchmarks will be finished.
More information about the core-libs-dev