Hello,
+1 for that change!
Small note :)
IMHO we can remove copyTo method from DefaultSelectionKeyHandler,
right, there is no reason to override it?
Thanks.
WBR,
Alexey.
On 19.02.2008, at 17:53, charlie hunt wrote:
> Zapping some of the non-relevant stuff to shrink message size.
>
> Responses embedded below.
>
> Jeanfrancois Arcand wrote:
>> charlie hunt wrote:
>>> ...[other stuff zapped]...
>>>
>>> 2.) I am not 100% happy with the refactoring since the
>>> SelectionKeyHandler interface implies expiring keys with its
>>> expire() method. And, this notion also propagates into
>>> TCPSelectorHandler.postSelect(). To be 100% happy, a
>>> SelectionKeyHandler interface and TCPSelectorHandler.postSelect()
>>> would need to be refactored. I don't have a problem with
>>> refactoring TCPSelectorHandler, but I don't like the idea of
>>> refactoring SelectionKeyHandler. I would summarize the issue here
>>> as; the notion of expiring keys should be a more general "post
>>> select" activity and not forced upon a user of a Grizzly transport
>>> when he / she does not need expiring keys support. We might
>>> consider addressing this in Grizzly 2.0 also.
>>
>> Interesting :-) Can you file an issue so we don't forget it?
>
> Yes. I am planning to submit an issue and also reference it the
> issue I have filed for this enhancement work.
>
>>
>> ... [stuff zapped] ...
>>
>> Comment inline...
>>
>> ... [stuff zapped] ...
>>>
>>> Index: main/java/com/sun/grizzly/TCPSelectorHandler.java
>>> ===================================================================
>>> --- main/java/com/sun/grizzly/TCPSelectorHandler.java (revision
>>> 840)
>>> +++ main/java/com/sun/grizzly/TCPSelectorHandler.java (working
>>> copy)
>>> @@ -407,7 +407,7 @@
>>> SelectionKey.OP_CONNECT);
>>>
>>> key.attach(CallbackHandlerSelectionKeyAttachment.create(key,
>>> callbackHandler));
>>> - boolean isConnected = false;
>>> + boolean isConnected;
>>
>> Stupid question. Not initializing the variable makes a difference?
>
> No such thing as a stupid question. ;-)
>
> It "can" make a difference. It depends on the usage and what else
> the JIT compiler observes in the usage of the application.
>
> It may in some cases make no difference. In some situations the JIT
> compiler may see that you are loading value into that variable
> before ever using the initialized value and essentially remove the
> initial "load" as an optimization. In other cases, it may not do
> that kind of optimization (remove the initial load). The only way
> to "really" know is to look at the generated code.
>
> It can be an issue if; the code resulting in a load instruction is
> on a really hot" path. But, since this specific case is an
> OP_CONNECT, it's likely this is will not a "big" difference, or a
> noticeable difference.
>
> I've just gotten into the habit of pro-actively removing
> "unnecessary loads" whenever I see them so they don't have an
> opportunity to become a problem.
>
> Btw, I'm not a JIT compiler expert. There's a lot of "rocket
> science" that goes on in the compiler and about the only I can
> really find out answers to what it is doing is look at the generated
> code in Sun Studio Collector / Analyzer in "Machine Mode". It's
> kind of a tedious task. But, interesting nonetheless.
>
>> ... [other stuff zapped] ...
>>
>> Some description :-)
>
> I'll add it.
>
>>> */
>>> public class BaseSelectionKeyHandler implements
>>> SelectionKeyHandler {
>>>
>>> ... [stuff zapped] ...
>>>
>>> @SuppressWarnings("empty-statement")
>>> protected void closeChannel(SelectableChannel channel) {
>>>
>>> if (channel instanceof SocketChannel) {
>>> Socket socket = ((SocketChannel) channel).socket();
>>> try{
>>> if (!socket.isInputShutdown())
>>> socket.shutdownInput();
>>> } catch (IOException ex){
>>> ;
>>> }
>>> try{
>>> if (!socket.isOutputShutdown())
>>> socket.shutdownOutput();
>>> } catch (IOException ex){
>>> ;
>>> }
>>> try{
>>> socket.close();
>>> } catch (IOException ex){
>>> ;
>>> }
>>> }
>>> try{
>>> channel.close();
>>> } catch (IOException ex){
>>> ; // LOG ME
>>> }
>> * @author Charlie Hunt
>>
>> Since we are refactoring, what do you think about logging all
>> IOException here to FINEST?
>
> Agreed. I was considering doing this. So, I'd be glad to do it.
>
> charlie ..
>
>>
>> Thanks!
>>
>> -- Jeanfrancois
>>
>>
>>> }
>>> }
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>>
>
> <
> charlie
> .hunt
> .vcf
> >---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net