Patch for com.sun.grizzlyController...

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Patch for com.sun.grizzlyController...

Erik Svensson-2
Howdy all!

In an effort to make my life easier I've patched Controller with two
AddProtocolFilter() methods and one RemoveProtocolFilter().
Code snippet below.
The addProtocolFilter():s check for the presence of a
ProtocolChainInstanceHandler and if there isn't one, creates a new
DefaultProtocolChainInstanceHandler and add the protocol filter to it.
A question: in ProtocolChain there are two add methods:

 public boolean addFilter(ProtocolFilter protocolFilter);
 public void addFilter(int pos, ProtocolFilter protocolFilter);

Should the second really return void, shouldn't it return boolean?


code snippet:

  public boolean addProtocolFilter(ProtocolFilter protocolFilter) {
    if (null == instanceHandler ) {
      instanceHandler = new DefaultProtocolChainInstanceHandler();
    }
    return instanceHandler.poll().addFilter(protocolFilter);
  }

  public void addProtocolFilter(ProtocolFilter protocolFilter, int position)
{
    if (null == instanceHandler) {
      instanceHandler = new DefaultProtocolChainInstanceHandler();
    }
    instanceHandler.poll().addFilter(position, protocolFilter);
  }

  public boolean removeProtocolFilter(ProtocolFilter protocolFilter) {
    if (instanceHandler != null) {
      return instanceHandler.poll().removeFilter(protocolFilter);
    }
    return false;
  }


cheers
Erik Svensson, SIX AB


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Patch for com.sun.grizzlyController...

Erik Svensson-2
Howdy all,

My patch below does not work if you want to add more than one protocol
filter to the chain. This is due to the behaviour of the
DefaultProtocolChainInstanceHandler. If DefaultProtocolChainInstanceHandler
is modified according to my previous mail it will work.

cheers

Erik Svensson, SIX AB

> Howdy all!
>
> In an effort to make my life easier I've patched Controller with two
> AddProtocolFilter() methods and one RemoveProtocolFilter().
> Code snippet below.
> The addProtocolFilter():s check for the presence of a
> ProtocolChainInstanceHandler and if there isn't one, creates a new
> DefaultProtocolChainInstanceHandler and add the protocol filter to it.
> A question: in ProtocolChain there are two add methods:
>
>  public boolean addFilter(ProtocolFilter protocolFilter);
>  public void addFilter(int pos, ProtocolFilter protocolFilter);
>
> Should the second really return void, shouldn't it return boolean?
>
>
> code snippet:
>
>   public boolean addProtocolFilter(ProtocolFilter protocolFilter) {
>     if (null == instanceHandler ) {
>       instanceHandler = new DefaultProtocolChainInstanceHandler();
>     }
>     return instanceHandler.poll().addFilter(protocolFilter);
>   }
>
>   public void addProtocolFilter(ProtocolFilter protocolFilter, int position)
> {
>     if (null == instanceHandler) {
>       instanceHandler = new DefaultProtocolChainInstanceHandler();
>     }
>     instanceHandler.poll().addFilter(position, protocolFilter);
>   }
>
>   public boolean removeProtocolFilter(ProtocolFilter protocolFilter) {
>     if (instanceHandler != null) {
>       return instanceHandler.poll().removeFilter(protocolFilter);
>     }
>     return false;
>   }
>
>
> cheers
> Erik Svensson, SIX AB
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Patch for com.sun.grizzlyController...

Jeanfrancois Arcand-2
In reply to this post by Erik Svensson-2


Erik Svensson wrote:

> Howdy all!
>
> In an effort to make my life easier I've patched Controller with two
> AddProtocolFilter() methods and one RemoveProtocolFilter().
> Code snippet below.
> The addProtocolFilter():s check for the presence of a
> ProtocolChainInstanceHandler and if there isn't one, creates a new
> DefaultProtocolChainInstanceHandler and add the protocol filter to it.
> A question: in ProtocolChain there are two add methods:
>
>  public boolean addFilter(ProtocolFilter protocolFilter);
>  public void addFilter(int pos, ProtocolFilter protocolFilter);
>
> Should the second really return void, shouldn't it return boolean?

Agree it should return boolean :-)


>
>
> code snippet:
>
>   public boolean addProtocolFilter(ProtocolFilter protocolFilter) {
>     if (null == instanceHandler ) {
>       instanceHandler = new DefaultProtocolChainInstanceHandler();
>     }
>     return instanceHandler.poll().addFilter(protocolFilter);
>   }
>
>   public void addProtocolFilter(ProtocolFilter protocolFilter, int position)
> {
>     if (null == instanceHandler) {
>       instanceHandler = new DefaultProtocolChainInstanceHandler();
>     }
>     instanceHandler.poll().addFilter(position, protocolFilter);
>   }
>
>   public boolean removeProtocolFilter(ProtocolFilter protocolFilter) {
>     if (instanceHandler != null) {
>       return instanceHandler.poll().removeFilter(protocolFilter);
>     }
>     return false;
>   }

I agree it might be useful and will reduce the number of code needed.
The only things we need to stress here is the
DefaultProtocolChainInstanceHandler is stateful, meaning every time the
Controller invokes it, you are getting a ProtocolChain instance per
Thread. Most of the time, you don't need to have one ProtocolChain per
Thread, but a single one where the ProtocolFilter are stateless as well.
That reduce the number of instance stacking on the heap under high load.

Can you send your complete diff ? Also do you think/have time to write a
unit test we can add under grizly/sec/test that automatically test your
proposal?

Thanks!

-- Jeanfrancois




>
>
> cheers
> Erik Svensson, SIX AB
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]