Hi Jeanfrancois,
Jeanfrancois wrote:
> This is strange as it looks for ' ', which is why start needs to be
> bumped by 1 so we start at /. Not sure we should remove the +1 unless
> I'm missing something.
About (1), I would like to explain more.
Assuming that a user request "GET /a.html HTTP/1.1",
---
Byte Index
G 0
E 1
T 2
(blank) 3
/ 4
a 5
. 6
h 7
t 8
m 9
l 10
(blank) 11
...
---
while(byteBuffer.hasRemaining()) {
c = byteBuffer.get();
switch(state) {
case 0:
if (c==0x20){
state = 1;
//----------------------------------(a) At this point, byteBuffer.position() is not 3 but 4
start = byteBuffer.position() + 1;
}
break;
case 1:
if (c==0x20){
//----------------------------------(b) At this point, byteBuffer.position() is not 11 but 12
end = byteBuffer.position() - 1;
byteBuffer.position(start);
byteBuffer.limit(end);
...
}
}
...
}
---
Above (a), after byteBuffer.get(), byteBuffer's position is already increased. So we need not to increase 1.
And also see the following:
---
protected String getContextRoot(ByteBuffer byteBuffer) {
byte[] chars = new byte[byteBuffer.limit() - byteBuffer.position()]; ------ (c) byteBuffer.position() is equal to 5 and byteBuffer.limit() is equal to 11
byteBuffer.get(chars);
...
}
---
Above (c), "byteBuffer.limit() - byteBuffer.position()" is equal to 6 and byteBuffer.position() is equal to 5.
So we will not obtain "/a.html" but "a.html".
Jeanfrancois wrote:
> Agree. But if we go that route we will need to improve the mapping of
> the request. So I think we will need to inject the Mapper or (I can see
> Hubert and Alexey smiling) write a new one to replace the over
> complicated Mapper.
Right. But I am afraid of the performance decline because of additional comparison logic. But I think Mapper is still needed for practical manner.
Jeanfrancois wrote:
> No you are totally right. I'm just curious about (1). For 3, do you want
> to go ahead and write/use the Mapper?
Sure. First of all, I would like to look into the Mapper. :)
After that, I will reply again.
Thanks!
--
Bongjae Chang
----- Original Message -----
From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
To: <dev_at_grizzly.dev.java.net>
Sent: Friday, July 17, 2009 11:29 PM
Subject: Re: About Grizzly's RCM
> Salut,
>
> Bongjae Chang wrote:
>> Hi,
>>
>> Today, I tried to review and test the ResourceAllocationFilter.
>
> Great!
>
>>
>> But, I have some questions about ResourceAllocationFilter.
>>
>>
>> 1. ResourceAllocationFilter#getContextRoot()'s return value
>>
>> If a client sends "GET /examples/index.html HTTP/1.1" request to the
>> grizzly server, getContextRoot() returns "examples/index.html".
>>
>> But, I think that ResourceAllocationFilter#getContextRoot() should
>> return "/examples/index.html" because a user's policyMetric is like
>> "/myApplication|0.9".
>
> Agree
>
>
>>
>> Is it right?
>>
>> If my thoughts are correct, here is the proposed patch.
>>
>> Index: com/sun/grizzly/rcm/ResourceAllocationFilter.java
>> ===================================================================
>> --- com/sun/grizzly/rcm/ResourceAllocationFilter.java (revision 3411)
>> +++ com/sun/grizzly/rcm/ResourceAllocationFilter.java (working copy)
>> @@ -466,7 +466,7 @@
>> case 0: // Search for first ' '
>> if (c == 0x20){
>> state = 1;
>> - start = byteBuffer.position() + 1;
>> + start = byteBuffer.position();
>> }
>> break;
>> case 1: // Search for next ' '
>>
>
> This is strange as it looks for ' ', which is why start needs to be
> bumped by 1 so we start at /. Not sure we should remove the +1 unless
> I'm missing something.
>
>>
>>
>> 2. ResourceAllocationFilter#invokeProtocolParser()'s strange code?
>>
>> Here is ResourceAllocationFilter#invokeProtocolParser()
>>
>> ---
>> public boolean invokeProtocolParser(...) {
>> ...
>> try{
>> NIOContext copyContext = (NIOContext) Cloner.clone(ctx);
>> copyContext.execute(copyContext.getProtocolChainContextTask());
>> *copyContext.setThreadPool(threadPool);*
>> } catch(...){
>> ...
>> }
>> ...
>> }
>> ---
>>
>> I couldn't understand the above code because the decided thread
>> pool(threadPool) was set after NIOContext.execute().
>>
>> Then, NIOContext.execute() doesn't use the decided thread pool but
>> original thread pool.
>>
>> Is it right?
>
> Wow this is seriously broken. You are right.
>
>
>>
>>
>> 3. Implications of the policyMetric
>>
>> When I read the JavaDoc and
>> http://weblogs.java.net/blog/jfarcand/archive/2006/04/resource_consum.html,
>> it seems that an user can set the policyMetric like the following.
>>
>> ---
>> -Dcom.sun.grizzly.rcm.policyMetric="/examples|0.9"
>> ---
>>
>> Then, does it mean that the policy is applied to all sub requests of
>> "examples" context? Or does it mean that the policy is applied to only
>> "/examples" request?
>>
>> I think that it is better that the policy should be applied to
>> "/examples", "/examples/index.html", "/examples/something" and etc...
>>
>> What do you think?
>
> Agree. But if we go that route we will need to improve the mapping of
> the request. So I think we will need to inject the Mapper or (I can see
> Hubert and Alexey smiling) write a new one to replace the over
> complicated Mapper.
>
>>
>>
>> If I am missing some points, please advice me.
>
> No you are totally right. I'm just curious about (1). For 3, do you want
> to go ahead and write/use the Mapper?
>
> A+
>
> - Jeanfrancois
>
>
>>
>> Thanks.
>>
>> --
>> Bongjae Chang
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
>
>