Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-07 Thread Philippe Mouawad
Hello Sebb,
By proxy do you mean:
Proxy.newProxyInstance and InvocationHandler, if so aren't we missing an
Interface (Sampler does not do the job and it does not use Entry parameter).
If so I agree lot of job.

Regarding the need to clone sampler, why do we need this, is there some data
that may be corrupted except CookieManager ?
I implemented the approach with CookieManager in ThreadLocal and it works
without need for thread safety.
I will attach it to ISSUE to let you review it.

Regards
Philippe

On Fri, Oct 7, 2011 at 2:46 AM, sebb seb...@gmail.com wrote:

 On 7 October 2011 00:00, sebb seb...@gmail.com wrote:
  On 6 October 2011 22:17, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  Hello Sebb,
  My responses below, there is one remaining point and I think we can go
 for
  implementation (i can do it if you agree).
 
  Regards
  Philippe
 
  On Thu, Oct 6, 2011 at 12:01 AM, sebb seb...@gmail.com wrote:
 
  On 5 October 2011 21:46, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   Hello Sebb, all,
   First a note regarding
   If cookies are being ignored, then the cookie manager property can
 just
  be
   cleared - i.e. there is no cookie manager.
  
   Just to be sure I understand, you mean Cookies of embedded resources
 are
   not used, because download of embedded resources may require
 JSESSIONID
   cookie at least or any cookie containing Session information.
   In this case can CookieManager be null or shouln't it be cloned from
  parent
   but it's result not used ? Maybe that's what you mean or I missed
  something.
 
  I forgot that cookies might be needed to access the embedded
  resources, so that won't work.
 
   So, regarding your last comment on 51919 here is an updated
  implementation
   proposition:
  
 - Add an option embedded_resources_use_cookies to use Cookies we
 get
 from embedded resources (conc or serial) download,  default value
 will
  be
 true (to handle frames by default).
 
  I don't think we need the option.
 
  OK
 
 
 - Create a Bean AsynSamplerResultHolder that will hold:
- HTTPSampleResult
- Cookie[]
 
  Yes, something like that.
 
  OK
 
 
   *Conc download:*
  
 - Pass CookieManager to AsynSampler, clone it and add existing
 cookie,
 and store it in ThreadLocal
 
  No need; if the cookie manager is cloned it can be stored in the normal
  place.
 
  I don't think so, because if we clone and set in normal place (call
  setCookieManager() in call() method of AsyncSampler  ) as we are
  calling
  HTTPSamplerBase.this.sample(...), we are working on same instance as
  Parent
  Sampler (that runs conc)  it will take place of parent Cookie Manager.
  Either I didn't get what you mean otherwise I think we must store it in
  ThreadLocal
 
  We need to clone the sampler and the cookie manager.
 
  AsynchSampler needs to be static; it can be passed this when it is
 created.
  Its constructor then clones this, and replaces the Cookie Manager (if
  any) with a copy of the CM, and copy the cookies as well.
 
  Probably need to add a copy constuctor to CM to make this easier.
 
  AsyncSampler then uses the cloned sampler to access the sample method.
 
  I'll do the first bit shortly - converting the AsychSampler to a
  static class - and you can do the rest.

 Unfortunately, cloning the sampler is not easy.
 I've had a look at wrapping the sampler to intercept calls to
 getCookieManager() but AFAICT that would require writing an
 interceptor proxy.
 Which is potentially a lot of work.

 I need to give this some more thought.

 
 - At end of sample:
- If embedded_resources_use_cookies is false =build
   AsynSamplerResultHolder
with HTTPSampleResult and empty cookies array
- If embedded_resources_use_cookies is true =build
   AsynSamplerResultHolder
with HTTPSampleResult and new cookies
 - Inside FutureResult#get loop, merge result in CookieManager
 
  Yes.
 
  OK, getCookieManager.add() will do the job.
 
 
   *Serial Mode:
   *
  
 - Before start of sample, backup CookieManager
 - At end of sample:
- If embedded_resources_use_cookies is false = ignore cookies
- If embedded_resources_use_cookies is true = add cookies to
  backup
CookieManager
 
  No need.
 
  OK
 
 
  
   Modify HTTPSamplerBase#getCookieManager to get CookieManager first
 from
   Thread Local then from testElement. (as today)
  
  
   Waiting for your comments on this.
 
  I was thinking that ignoring cookies would simplify the problem, but
  it does not seem to.
 
  So the only change we would need to do is to clone the cookies for
  each task, and collect them again at the end.
 
  No need to make the cookie storage class thread-safe.
 
  OK
 
 
   Regards
   Philippe
  
   On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 15:39, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
   

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-07 Thread sebb
On 7 October 2011 09:45, Philippe Mouawad philippe.moua...@gmail.com wrote:
 Hello Sebb,
 By proxy do you mean:
 Proxy.newProxyInstance and InvocationHandler, if so aren't we missing an
 Interface (Sampler does not do the job and it does not use Entry parameter).
 If so I agree lot of job.

Yes.

 Regarding the need to clone sampler, why do we need this, is there some data
 that may be corrupted except CookieManager ?

I don't know.
We already discovered problems with CM and CacheManager, but there are
a lot of other data areas that could potentially be affected.
For example, the HC3 and HC4 clients and their connections.

As I wrote earlier, the JMeter design is based on sampler classes
being single-threaded.

Just had a quick look, and for example the HC4 currentRequest (used
for interrupting the sampler) is a potential problem.
Any instance data that can be changed by the sampling process may
cause problems.

 I implemented the approach with CookieManager in ThreadLocal and it works
 without need for thread safety.
 I will attach it to ISSUE to let you review it.

OK, I'll have a look.

However, as I just wrote, I suspect this will only fix the issue we know about.

 Regards
 Philippe

 On Fri, Oct 7, 2011 at 2:46 AM, sebb seb...@gmail.com wrote:

 On 7 October 2011 00:00, sebb seb...@gmail.com wrote:
  On 6 October 2011 22:17, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  Hello Sebb,
  My responses below, there is one remaining point and I think we can go
 for
  implementation (i can do it if you agree).
 
  Regards
  Philippe
 
  On Thu, Oct 6, 2011 at 12:01 AM, sebb seb...@gmail.com wrote:
 
  On 5 October 2011 21:46, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   Hello Sebb, all,
   First a note regarding
   If cookies are being ignored, then the cookie manager property can
 just
  be
   cleared - i.e. there is no cookie manager.
  
   Just to be sure I understand, you mean Cookies of embedded resources
 are
   not used, because download of embedded resources may require
 JSESSIONID
   cookie at least or any cookie containing Session information.
   In this case can CookieManager be null or shouln't it be cloned from
  parent
   but it's result not used ? Maybe that's what you mean or I missed
  something.
 
  I forgot that cookies might be needed to access the embedded
  resources, so that won't work.
 
   So, regarding your last comment on 51919 here is an updated
  implementation
   proposition:
  
     - Add an option embedded_resources_use_cookies to use Cookies we
 get
     from embedded resources (conc or serial) download,  default value
 will
  be
     true (to handle frames by default).
 
  I don't think we need the option.
 
  OK
 
 
     - Create a Bean AsynSamplerResultHolder that will hold:
        - HTTPSampleResult
        - Cookie[]
 
  Yes, something like that.
 
  OK
 
 
   *Conc download:*
  
     - Pass CookieManager to AsynSampler, clone it and add existing
 cookie,
     and store it in ThreadLocal
 
  No need; if the cookie manager is cloned it can be stored in the normal
  place.
 
  I don't think so, because if we clone and set in normal place (call
  setCookieManager() in call() method of AsyncSampler  ) as we are
  calling
  HTTPSamplerBase.this.sample(...), we are working on same instance as
  Parent
  Sampler (that runs conc)  it will take place of parent Cookie Manager.
  Either I didn't get what you mean otherwise I think we must store it in
  ThreadLocal
 
  We need to clone the sampler and the cookie manager.
 
  AsynchSampler needs to be static; it can be passed this when it is
 created.
  Its constructor then clones this, and replaces the Cookie Manager (if
  any) with a copy of the CM, and copy the cookies as well.
 
  Probably need to add a copy constuctor to CM to make this easier.
 
  AsyncSampler then uses the cloned sampler to access the sample method.
 
  I'll do the first bit shortly - converting the AsychSampler to a
  static class - and you can do the rest.

 Unfortunately, cloning the sampler is not easy.
 I've had a look at wrapping the sampler to intercept calls to
 getCookieManager() but AFAICT that would require writing an
 interceptor proxy.
 Which is potentially a lot of work.

 I need to give this some more thought.

 
     - At end of sample:
        - If embedded_resources_use_cookies is false =build
   AsynSamplerResultHolder
        with HTTPSampleResult and empty cookies array
        - If embedded_resources_use_cookies is true =build
   AsynSamplerResultHolder
        with HTTPSampleResult and new cookies
     - Inside FutureResult#get loop, merge result in CookieManager
 
  Yes.
 
  OK, getCookieManager.add() will do the job.
 
 
   *Serial Mode:
   *
  
     - Before start of sample, backup CookieManager
     - At end of sample:
        - If embedded_resources_use_cookies is false = ignore cookies
        - If embedded_resources_use_cookies is true = add cookies to
  backup
        CookieManager
 
  No need.
 
  OK
 
 
  
 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-06 Thread Philippe Mouawad
Hello Sebb,
My responses below, there is one remaining point and I think we can go for
implementation (i can do it if you agree).

Regards
Philippe

On Thu, Oct 6, 2011 at 12:01 AM, sebb seb...@gmail.com wrote:

 On 5 October 2011 21:46, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  Hello Sebb, all,
  First a note regarding
  If cookies are being ignored, then the cookie manager property can just
 be
  cleared - i.e. there is no cookie manager.
 
  Just to be sure I understand, you mean Cookies of embedded resources are
  not used, because download of embedded resources may require JSESSIONID
  cookie at least or any cookie containing Session information.
  In this case can CookieManager be null or shouln't it be cloned from
 parent
  but it's result not used ? Maybe that's what you mean or I missed
 something.

 I forgot that cookies might be needed to access the embedded
 resources, so that won't work.

  So, regarding your last comment on 51919 here is an updated
 implementation
  proposition:
 
- Add an option embedded_resources_use_cookies to use Cookies we get
from embedded resources (conc or serial) download,  default value will
 be
true (to handle frames by default).

 I don't think we need the option.

OK


- Create a Bean AsynSamplerResultHolder that will hold:
   - HTTPSampleResult
   - Cookie[]

 Yes, something like that.

OK


  *Conc download:*
 
- Pass CookieManager to AsynSampler, clone it and add existing cookie,
and store it in ThreadLocal

 No need; if the cookie manager is cloned it can be stored in the normal
 place.

 I don't think so, because if we clone and set in normal place (call
setCookieManager() in call() method of AsyncSampler  ) as we are  calling
HTTPSamplerBase.this.sample(...), we are working on same instance as  Parent
Sampler (that runs conc)  it will take place of parent Cookie Manager.
Either I didn't get what you mean otherwise I think we must store it in
ThreadLocal


- At end of sample:
   - If embedded_resources_use_cookies is false =build
  AsynSamplerResultHolder
   with HTTPSampleResult and empty cookies array
   - If embedded_resources_use_cookies is true =build
  AsynSamplerResultHolder
   with HTTPSampleResult and new cookies
- Inside FutureResult#get loop, merge result in CookieManager

 Yes.

OK, getCookieManager.add() will do the job.


  *Serial Mode:
  *
 
- Before start of sample, backup CookieManager
- At end of sample:
   - If embedded_resources_use_cookies is false = ignore cookies
   - If embedded_resources_use_cookies is true = add cookies to
 backup
   CookieManager

 No need.

OK


 
  Modify HTTPSamplerBase#getCookieManager to get CookieManager first from
  Thread Local then from testElement. (as today)
 
 
  Waiting for your comments on this.

 I was thinking that ignoring cookies would simplify the problem, but
 it does not seem to.

 So the only change we would need to do is to clone the cookies for
 each task, and collect them again at the end.

 No need to make the cookie storage class thread-safe.

 OK


  Regards
  Philippe
 
  On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 14:04, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
You are right,
Patch was just about quick fix before the more impacting fix.
   
Here are my propositions regarding this more impacting fix:
   
  - Add an option to make conc download use or not cookie, default
  value
  will be false.
  - In AsyncSampler make a Clone with current cookies of Parent
  CookieManager (the one that is calling Executor) and store it in
   ThreadLocal
  - Change HttpSamplerBase#getCookieManager to retrieve first in
  Thread
  Local then in instance
  - If option is true, when reading res in FutureHTTPSampleResult
  loop,
  reinject cookies inside ParentCookie otherwise ignore them
  
   As I wrote earlier, I'm not sure it ever makes sense to handle
 cookies
   from embedded resources, so it would be simpler just to ignore them.
  
   I am not sure Frame couldn't be concerned by this case, so in my
 opinion
  it
   should be a parameter not ignored by default.
 
  I'd overlooked frame.
 
  
   I'm looking at passing the current sampler to the AsyncSampler class,
   which can then clone it.
   I think that will create an independent set of cookies, so there can
   be no need to protect against concurrent updates.
  
   Agree, that's how I imagined it, but then you agree you have to store
   CookieManager in thread local ?
 
  If cookies are being ignored, then the cookie manager property can
  just be cleared - i.e. there is no cookie manager.
 
  Alternatively, it will also need to be cloned.
 
  
   We then need to consider if non-concurrent downloads should still be
   

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-05 Thread Philippe Mouawad
Hello Sebb, all,
First a note regarding
If cookies are being ignored, then the cookie manager property can just be
cleared - i.e. there is no cookie manager.

Just to be sure I understand, you mean Cookies of embedded resources are
not used, because download of embedded resources may require JSESSIONID
cookie at least or any cookie containing Session information.
In this case can CookieManager be null or shouln't it be cloned from parent
but it's result not used ? Maybe that's what you mean or I missed something.

So, regarding your last comment on 51919 here is an updated implementation
proposition:

   - Add an option embedded_resources_use_cookies to use Cookies we get
   from embedded resources (conc or serial) download,  default value will be
   true (to handle frames by default).
   - Create a Bean AsynSamplerResultHolder that will hold:
  - HTTPSampleResult
  - Cookie[]

*Conc download:*

   - Pass CookieManager to AsynSampler, clone it and add existing cookie,
   and store it in ThreadLocal
   - At end of sample:
  - If embedded_resources_use_cookies is false =build
AsynSamplerResultHolder
  with HTTPSampleResult and empty cookies array
  - If embedded_resources_use_cookies is true =build
AsynSamplerResultHolder
  with HTTPSampleResult and new cookies
   - Inside FutureResult#get loop, merge result in CookieManager

*Serial Mode:
*

   - Before start of sample, backup CookieManager
   - At end of sample:
  - If embedded_resources_use_cookies is false = ignore cookies
  - If embedded_resources_use_cookies is true = add cookies to backup
  CookieManager


Modify HTTPSamplerBase#getCookieManager to get CookieManager first from
Thread Local then from testElement. (as today)


Waiting for your comments on this.

Regards
Philippe

On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   You are right,
   Patch was just about quick fix before the more impacting fix.
  
   Here are my propositions regarding this more impacting fix:
  
 - Add an option to make conc download use or not cookie, default
 value
 will be false.
 - In AsyncSampler make a Clone with current cookies of Parent
 CookieManager (the one that is calling Executor) and store it in
  ThreadLocal
 - Change HttpSamplerBase#getCookieManager to retrieve first in
 Thread
 Local then in instance
 - If option is true, when reading res in FutureHTTPSampleResult
 loop,
 reinject cookies inside ParentCookie otherwise ignore them
 
  As I wrote earlier, I'm not sure it ever makes sense to handle cookies
  from embedded resources, so it would be simpler just to ignore them.
 
  I am not sure Frame couldn't be concerned by this case, so in my opinion
 it
  should be a parameter not ignored by default.

 I'd overlooked frame.

 
  I'm looking at passing the current sampler to the AsyncSampler class,
  which can then clone it.
  I think that will create an independent set of cookies, so there can
  be no need to protect against concurrent updates.
 
  Agree, that's how I imagined it, but then you agree you have to store
  CookieManager in thread local ?

 If cookies are being ignored, then the cookie manager property can
 just be cleared - i.e. there is no cookie manager.

 Alternatively, it will also need to be cloned.

 
  We then need to consider if non-concurrent downloads should still be
  processed for cookies.
 
 
  Conc and serial should have the same behaviour

 
  PS : Does it mean that you are working on issue and will fix it on your
 side

 Not yet decided; we need to agree on the approach first.

 
 
   Regards
   Philippe
  
   On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 13:14, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
Sebb,
Do you want me to provide a patch with ConcurrentHashMap where I
 will
   have
to handle null keys and values (not same behaviour as HashMap) or
 we
   forget
about this approach ?
  
   I don't think we have yet decided how best to handle concurrent
  downloads.
  
   e.g. should they even be setting cookies?
  
Regards
Philippe
   
On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
   philippe.moua...@gmail.com
wrote:
   
Hello,
Just a little update on my test.
I added a clear and gc before each Map instanciation and results
 are
different:
   
   - HashMapput:645
   - ConcurrentHashMapput:832
   - ConcurrentReaderHashMapput:620
   - HashMap get:17
   - ConcurrentHashMap get:32
   - ConcurrentReaderHashMap get:60
   
   
So it kind of closes the debate, sorry for disturbance.
Regards
Philippe
   
   
   
   
public class TestMap {
   
public static void main(String[] args) {
  

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Rainer Jung
On 02.10.2011 23:17, Philippe Mouawad wrote:
 Ok, hope we can do the same.

See https://issues.apache.org/jira/browse/XMLBEANS-447

We are not the only people, who doubt it's correct to include that class ...

There was also a discussion some time ago in another ASF project,
because the Sun license which is cited by Doug Lea has a no use in
nuclear facilities clause, which make it non-usable as an Open Source
license.

It looks like we are stuck here.

And the mentioning of the Harmony class in the above cited issue is a
red herring. Diffing it with the JDK 5 standard ConcurrentHashMap shows
little difference, so I doubt it will be much faster (though I didn't
inspect the delta in detail).

 I must say I don't understand why ConcurrentReaderHashMap is not in JDK.

There's a discussion list for JSR166 (the concurrency stuff lead by Doug
Lea) mentioned on the JSR 166 page maintained by Doug Lea. So maybe you
can post a technical question there (what's the right class that solve
the following problem ... and doesn't have sun licensing restrictions).

Regards,

Rainer


-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread sebb
On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de wrote:
 On 02.10.2011 23:17, Philippe Mouawad wrote:
 Ok, hope we can do the same.

 See https://issues.apache.org/jira/browse/XMLBEANS-447

 We are not the only people, who doubt it's correct to include that class ...

 There was also a discussion some time ago in another ASF project,
 because the Sun license which is cited by Doug Lea has a no use in
 nuclear facilities clause, which make it non-usable as an Open Source
 license.

 It looks like we are stuck here.

Yes, apart from the binary option which brings in lots of unneeded code.

 And the mentioning of the Harmony class in the above cited issue is a
 red herring. Diffing it with the JDK 5 standard ConcurrentHashMap shows
 little difference, so I doubt it will be much faster (though I didn't
 inspect the delta in detail).

I think the Harmony class was only mentioned as a means of supporting
Java 1.4, not as an alternative faster implementation.

 I must say I don't understand why ConcurrentReaderHashMap is not in JDK.

 There's a discussion list for JSR166 (the concurrency stuff lead by Doug
 Lea) mentioned on the JSR 166 page maintained by Doug Lea. So maybe you
 can post a technical question there (what's the right class that solve
 the following problem ... and doesn't have sun licensing restrictions).

 Regards,

 Rainer


 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Philippe Mouawad
Hello,
Just a little update on my test.
I added a clear and gc before each Map instanciation and results are
different:

   - HashMapput:645
   - ConcurrentHashMapput:832
   - ConcurrentReaderHashMapput:620
   - HashMap get:17
   - ConcurrentHashMap get:32
   - ConcurrentReaderHashMap get:60


So it kind of closes the debate, sorry for disturbance.
Regards
Philippe



public class TestMap {

public static void main(String[] args) {
StopWatch timer = new StopWatch();

Map map = new HashMap();
testPut(timer, map, HashMap);
timer.reset();

map.clear();
System.gc();
map = new ConcurrentHashMap();
testPut(timer, map, ConcurrentHashMap);
timer.reset();

map.clear();
System.gc();
map = new ConcurrentReaderHashMap();
testPut(timer, map, ConcurrentReaderHashMap);
timer.reset();


map.clear();
System.gc();
map = new HashMap();
testGet(timer, map, HashMap);
timer.reset();

map.clear();
System.gc();
map = new ConcurrentHashMap();
testGet(timer, map, ConcurrentHashMap);
timer.reset();

map.clear();
System.gc();
map = new ConcurrentReaderHashMap();
testGet(timer, map, ConcurrentReaderHashMap);
timer.reset();
}

/**
 * @param timer
 */
private static void testGet(StopWatch timer, Map map, String type) {
timer.start();
for (int i = 0; i  100; i++) {
map.get(i);
}
timer.stop();
System.out.println(type+ get:+timer.getTime());
}

/**
 * @param timer
 */
private static void testPut(StopWatch timer, Map map, String type) {
timer.start();
for (int i = 0; i  100; i++) {
map.put(i,i);
}
timer.stop();
System.out.println(type+put:+timer.getTime());
}


Regards
Philippe

On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de wrote:
  On 02.10.2011 23:17, Philippe Mouawad wrote:
  Ok, hope we can do the same.
 
  See https://issues.apache.org/jira/browse/XMLBEANS-447
 
  We are not the only people, who doubt it's correct to include that class
 ...
 
  There was also a discussion some time ago in another ASF project,
  because the Sun license which is cited by Doug Lea has a no use in
  nuclear facilities clause, which make it non-usable as an Open Source
  license.
 
  It looks like we are stuck here.

 Yes, apart from the binary option which brings in lots of unneeded code.

  And the mentioning of the Harmony class in the above cited issue is a
  red herring. Diffing it with the JDK 5 standard ConcurrentHashMap shows
  little difference, so I doubt it will be much faster (though I didn't
  inspect the delta in detail).

 I think the Harmony class was only mentioned as a means of supporting
 Java 1.4, not as an alternative faster implementation.

  I must say I don't understand why ConcurrentReaderHashMap is not in JDK.
 
  There's a discussion list for JSR166 (the concurrency stuff lead by Doug
  Lea) mentioned on the JSR 166 page maintained by Doug Lea. So maybe you
  can post a technical question there (what's the right class that solve
  the following problem ... and doesn't have sun licensing restrictions).
 
  Regards,
 
  Rainer
 
 
  -
  To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
  For additional commands, e-mail: dev-h...@jakarta.apache.org
 
 

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




-- 
Cordialement.
Philippe Mouawad.


Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Philippe Mouawad
Sebb,
Do you want me to provide a patch with ConcurrentHashMap where I will have
to handle null keys and values (not same behaviour as HashMap) or we forget
about this approach ?
Regards
Philippe

On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad philippe.moua...@gmail.com
 wrote:

 Hello,
 Just a little update on my test.
 I added a clear and gc before each Map instanciation and results are
 different:

- HashMapput:645
- ConcurrentHashMapput:832
- ConcurrentReaderHashMapput:620
- HashMap get:17
- ConcurrentHashMap get:32
- ConcurrentReaderHashMap get:60


 So it kind of closes the debate, sorry for disturbance.
 Regards
 Philippe




 public class TestMap {

 public static void main(String[] args) {
 StopWatch timer = new StopWatch();

 Map map = new HashMap();
 testPut(timer, map, HashMap);
 timer.reset();

 map.clear();
 System.gc();

 map = new ConcurrentHashMap();
 testPut(timer, map, ConcurrentHashMap);
 timer.reset();

 map.clear();
 System.gc();

 map = new ConcurrentReaderHashMap();
 testPut(timer, map, ConcurrentReaderHashMap);
 timer.reset();


 map.clear();
 System.gc();

 map = new HashMap();
 testGet(timer, map, HashMap);
 timer.reset();

 map.clear();
 System.gc();

 map = new ConcurrentHashMap();
 testGet(timer, map, ConcurrentHashMap);
 timer.reset();

 map.clear();
 System.gc();

 map = new ConcurrentReaderHashMap();
 testGet(timer, map, ConcurrentReaderHashMap);
 timer.reset();
 }

 /**
  * @param timer
  */
 private static void testGet(StopWatch timer, Map map, String type) {
 timer.start();
 for (int i = 0; i  100; i++) {
 map.get(i);
 }
 timer.stop();
 System.out.println(type+ get:+timer.getTime());
 }

 /**
  * @param timer
  */
 private static void testPut(StopWatch timer, Map map, String type) {
 timer.start();
 for (int i = 0; i  100; i++) {
 map.put(i,i);
 }
 timer.stop();
 System.out.println(type+put:+timer.getTime());
 }


 Regards
 Philippe


 On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de wrote:
  On 02.10.2011 23:17, Philippe Mouawad wrote:
  Ok, hope we can do the same.
 
  See https://issues.apache.org/jira/browse/XMLBEANS-447
 
  We are not the only people, who doubt it's correct to include that class
 ...
 
  There was also a discussion some time ago in another ASF project,
  because the Sun license which is cited by Doug Lea has a no use in
  nuclear facilities clause, which make it non-usable as an Open Source
  license.
 
  It looks like we are stuck here.

 Yes, apart from the binary option which brings in lots of unneeded code.

  And the mentioning of the Harmony class in the above cited issue is a
  red herring. Diffing it with the JDK 5 standard ConcurrentHashMap shows
  little difference, so I doubt it will be much faster (though I didn't
  inspect the delta in detail).

 I think the Harmony class was only mentioned as a means of supporting
 Java 1.4, not as an alternative faster implementation.

  I must say I don't understand why ConcurrentReaderHashMap is not in
 JDK.
 
  There's a discussion list for JSR166 (the concurrency stuff lead by Doug
  Lea) mentioned on the JSR 166 page maintained by Doug Lea. So maybe you
  can post a technical question there (what's the right class that solve
  the following problem ... and doesn't have sun licensing restrictions).
 
  Regards,
 
  Rainer
 
 
  -
  To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
  For additional commands, e-mail: dev-h...@jakarta.apache.org
 
 

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




 --
 Cordialement.
 Philippe Mouawad.






-- 
Cordialement.
Philippe Mouawad.


Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread sebb
On 3 October 2011 13:14, Philippe Mouawad philippe.moua...@gmail.com wrote:
 Sebb,
 Do you want me to provide a patch with ConcurrentHashMap where I will have
 to handle null keys and values (not same behaviour as HashMap) or we forget
 about this approach ?

I don't think we have yet decided how best to handle concurrent downloads.

e.g. should they even be setting cookies?

 Regards
 Philippe

 On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad philippe.moua...@gmail.com
 wrote:

 Hello,
 Just a little update on my test.
 I added a clear and gc before each Map instanciation and results are
 different:

    - HashMapput:645
    - ConcurrentHashMapput:832
    - ConcurrentReaderHashMapput:620
    - HashMap get:17
    - ConcurrentHashMap get:32
    - ConcurrentReaderHashMap get:60


 So it kind of closes the debate, sorry for disturbance.
 Regards
 Philippe




 public class TestMap {

     public static void main(String[] args) {
         StopWatch timer = new StopWatch();

         Map map = new HashMap();
         testPut(timer, map, HashMap);
         timer.reset();

         map.clear();
         System.gc();

         map = new ConcurrentHashMap();
         testPut(timer, map, ConcurrentHashMap);
         timer.reset();

         map.clear();
         System.gc();

         map = new ConcurrentReaderHashMap();
         testPut(timer, map, ConcurrentReaderHashMap);
         timer.reset();


         map.clear();
         System.gc();

         map = new HashMap();
         testGet(timer, map, HashMap);
         timer.reset();

         map.clear();
         System.gc();

         map = new ConcurrentHashMap();
         testGet(timer, map, ConcurrentHashMap);
         timer.reset();

         map.clear();
         System.gc();

         map = new ConcurrentReaderHashMap();
         testGet(timer, map, ConcurrentReaderHashMap);
         timer.reset();
     }

     /**
      * @param timer
      */
     private static void testGet(StopWatch timer, Map map, String type) {
         timer.start();
         for (int i = 0; i  100; i++) {
             map.get(i);
         }
         timer.stop();
         System.out.println(type+ get:+timer.getTime());
     }

     /**
      * @param timer
      */
     private static void testPut(StopWatch timer, Map map, String type) {
         timer.start();
         for (int i = 0; i  100; i++) {
             map.put(i,i);
         }
         timer.stop();
         System.out.println(type+put:+timer.getTime());
     }


 Regards
 Philippe


 On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de wrote:
  On 02.10.2011 23:17, Philippe Mouawad wrote:
  Ok, hope we can do the same.
 
  See https://issues.apache.org/jira/browse/XMLBEANS-447
 
  We are not the only people, who doubt it's correct to include that class
 ...
 
  There was also a discussion some time ago in another ASF project,
  because the Sun license which is cited by Doug Lea has a no use in
  nuclear facilities clause, which make it non-usable as an Open Source
  license.
 
  It looks like we are stuck here.

 Yes, apart from the binary option which brings in lots of unneeded code.

  And the mentioning of the Harmony class in the above cited issue is a
  red herring. Diffing it with the JDK 5 standard ConcurrentHashMap shows
  little difference, so I doubt it will be much faster (though I didn't
  inspect the delta in detail).

 I think the Harmony class was only mentioned as a means of supporting
 Java 1.4, not as an alternative faster implementation.

  I must say I don't understand why ConcurrentReaderHashMap is not in
 JDK.
 
  There's a discussion list for JSR166 (the concurrency stuff lead by Doug
  Lea) mentioned on the JSR 166 page maintained by Doug Lea. So maybe you
  can post a technical question there (what's the right class that solve
  the following problem ... and doesn't have sun licensing restrictions).
 
  Regards,
 
  Rainer
 
 
  -
  To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
  For additional commands, e-mail: dev-h...@jakarta.apache.org
 
 

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




 --
 Cordialement.
 Philippe Mouawad.






 --
 Cordialement.
 Philippe Mouawad.


-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Philippe Mouawad
You are right,
Patch was just about quick fix before the more impacting fix.

Here are my propositions regarding this more impacting fix:

   - Add an option to make conc download use or not cookie, default value
   will be false.
   - In AsyncSampler make a Clone with current cookies of Parent
   CookieManager (the one that is calling Executor) and store it in ThreadLocal
   - Change HttpSamplerBase#getCookieManager to retrieve first in Thread
   Local then in instance
   - If option is true, when reading res in FutureHTTPSampleResult loop,
   reinject cookies inside ParentCookie otherwise ignore them


Regards
Philippe

On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 13:14, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  Sebb,
  Do you want me to provide a patch with ConcurrentHashMap where I will
 have
  to handle null keys and values (not same behaviour as HashMap) or we
 forget
  about this approach ?

 I don't think we have yet decided how best to handle concurrent downloads.

 e.g. should they even be setting cookies?

  Regards
  Philippe
 
  On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
 philippe.moua...@gmail.com
  wrote:
 
  Hello,
  Just a little update on my test.
  I added a clear and gc before each Map instanciation and results are
  different:
 
 - HashMapput:645
 - ConcurrentHashMapput:832
 - ConcurrentReaderHashMapput:620
 - HashMap get:17
 - ConcurrentHashMap get:32
 - ConcurrentReaderHashMap get:60
 
 
  So it kind of closes the debate, sorry for disturbance.
  Regards
  Philippe
 
 
 
 
  public class TestMap {
 
  public static void main(String[] args) {
  StopWatch timer = new StopWatch();
 
  Map map = new HashMap();
  testPut(timer, map, HashMap);
  timer.reset();
 
  map.clear();
  System.gc();
 
  map = new ConcurrentHashMap();
  testPut(timer, map, ConcurrentHashMap);
  timer.reset();
 
  map.clear();
  System.gc();
 
  map = new ConcurrentReaderHashMap();
  testPut(timer, map, ConcurrentReaderHashMap);
  timer.reset();
 
 
  map.clear();
  System.gc();
 
  map = new HashMap();
  testGet(timer, map, HashMap);
  timer.reset();
 
  map.clear();
  System.gc();
 
  map = new ConcurrentHashMap();
  testGet(timer, map, ConcurrentHashMap);
  timer.reset();
 
  map.clear();
  System.gc();
 
  map = new ConcurrentReaderHashMap();
  testGet(timer, map, ConcurrentReaderHashMap);
  timer.reset();
  }
 
  /**
   * @param timer
   */
  private static void testGet(StopWatch timer, Map map, String type) {
  timer.start();
  for (int i = 0; i  100; i++) {
  map.get(i);
  }
  timer.stop();
  System.out.println(type+ get:+timer.getTime());
  }
 
  /**
   * @param timer
   */
  private static void testPut(StopWatch timer, Map map, String type) {
  timer.start();
  for (int i = 0; i  100; i++) {
  map.put(i,i);
  }
  timer.stop();
  System.out.println(type+put:+timer.getTime());
  }
 
 
  Regards
  Philippe
 
 
  On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de wrote:
   On 02.10.2011 23:17, Philippe Mouawad wrote:
   Ok, hope we can do the same.
  
   See https://issues.apache.org/jira/browse/XMLBEANS-447
  
   We are not the only people, who doubt it's correct to include that
 class
  ...
  
   There was also a discussion some time ago in another ASF project,
   because the Sun license which is cited by Doug Lea has a no use in
   nuclear facilities clause, which make it non-usable as an Open
 Source
   license.
  
   It looks like we are stuck here.
 
  Yes, apart from the binary option which brings in lots of unneeded
 code.
 
   And the mentioning of the Harmony class in the above cited issue is a
   red herring. Diffing it with the JDK 5 standard ConcurrentHashMap
 shows
   little difference, so I doubt it will be much faster (though I didn't
   inspect the delta in detail).
 
  I think the Harmony class was only mentioned as a means of supporting
  Java 1.4, not as an alternative faster implementation.
 
   I must say I don't understand why ConcurrentReaderHashMap is not in
  JDK.
  
   There's a discussion list for JSR166 (the concurrency stuff lead by
 Doug
   Lea) mentioned on the JSR 166 page maintained by Doug Lea. So maybe
 you
   can post a technical question there (what's the right class that
 solve
   the following problem ... and doesn't have sun licensing
 restrictions).
  
   Regards,
  
   Rainer
  
  
   -
   To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
   For additional 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread sebb
On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com wrote:
 You are right,
 Patch was just about quick fix before the more impacting fix.

 Here are my propositions regarding this more impacting fix:

   - Add an option to make conc download use or not cookie, default value
   will be false.
   - In AsyncSampler make a Clone with current cookies of Parent
   CookieManager (the one that is calling Executor) and store it in ThreadLocal
   - Change HttpSamplerBase#getCookieManager to retrieve first in Thread
   Local then in instance
   - If option is true, when reading res in FutureHTTPSampleResult loop,
   reinject cookies inside ParentCookie otherwise ignore them

As I wrote earlier, I'm not sure it ever makes sense to handle cookies
from embedded resources, so it would be simpler just to ignore them.

I'm looking at passing the current sampler to the AsyncSampler class,
which can then clone it.
I think that will create an independent set of cookies, so there can
be no need to protect against concurrent updates.

We then need to consider if non-concurrent downloads should still be
processed for cookies.


 Regards
 Philippe

 On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 13:14, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  Sebb,
  Do you want me to provide a patch with ConcurrentHashMap where I will
 have
  to handle null keys and values (not same behaviour as HashMap) or we
 forget
  about this approach ?

 I don't think we have yet decided how best to handle concurrent downloads.

 e.g. should they even be setting cookies?

  Regards
  Philippe
 
  On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
 philippe.moua...@gmail.com
  wrote:
 
  Hello,
  Just a little update on my test.
  I added a clear and gc before each Map instanciation and results are
  different:
 
     - HashMapput:645
     - ConcurrentHashMapput:832
     - ConcurrentReaderHashMapput:620
     - HashMap get:17
     - ConcurrentHashMap get:32
     - ConcurrentReaderHashMap get:60
 
 
  So it kind of closes the debate, sorry for disturbance.
  Regards
  Philippe
 
 
 
 
  public class TestMap {
 
      public static void main(String[] args) {
          StopWatch timer = new StopWatch();
 
          Map map = new HashMap();
          testPut(timer, map, HashMap);
          timer.reset();
 
          map.clear();
          System.gc();
 
          map = new ConcurrentHashMap();
          testPut(timer, map, ConcurrentHashMap);
          timer.reset();
 
          map.clear();
          System.gc();
 
          map = new ConcurrentReaderHashMap();
          testPut(timer, map, ConcurrentReaderHashMap);
          timer.reset();
 
 
          map.clear();
          System.gc();
 
          map = new HashMap();
          testGet(timer, map, HashMap);
          timer.reset();
 
          map.clear();
          System.gc();
 
          map = new ConcurrentHashMap();
          testGet(timer, map, ConcurrentHashMap);
          timer.reset();
 
          map.clear();
          System.gc();
 
          map = new ConcurrentReaderHashMap();
          testGet(timer, map, ConcurrentReaderHashMap);
          timer.reset();
      }
 
      /**
       * @param timer
       */
      private static void testGet(StopWatch timer, Map map, String type) {
          timer.start();
          for (int i = 0; i  100; i++) {
              map.get(i);
          }
          timer.stop();
          System.out.println(type+ get:+timer.getTime());
      }
 
      /**
       * @param timer
       */
      private static void testPut(StopWatch timer, Map map, String type) {
          timer.start();
          for (int i = 0; i  100; i++) {
              map.put(i,i);
          }
          timer.stop();
          System.out.println(type+put:+timer.getTime());
      }
 
 
  Regards
  Philippe
 
 
  On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de wrote:
   On 02.10.2011 23:17, Philippe Mouawad wrote:
   Ok, hope we can do the same.
  
   See https://issues.apache.org/jira/browse/XMLBEANS-447
  
   We are not the only people, who doubt it's correct to include that
 class
  ...
  
   There was also a discussion some time ago in another ASF project,
   because the Sun license which is cited by Doug Lea has a no use in
   nuclear facilities clause, which make it non-usable as an Open
 Source
   license.
  
   It looks like we are stuck here.
 
  Yes, apart from the binary option which brings in lots of unneeded
 code.
 
   And the mentioning of the Harmony class in the above cited issue is a
   red herring. Diffing it with the JDK 5 standard ConcurrentHashMap
 shows
   little difference, so I doubt it will be much faster (though I didn't
   inspect the delta in detail).
 
  I think the Harmony class was only mentioned as a means of supporting
  Java 1.4, not as an alternative faster implementation.
 
   I must say I don't understand why 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Philippe Mouawad
On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  You are right,
  Patch was just about quick fix before the more impacting fix.
 
  Here are my propositions regarding this more impacting fix:
 
- Add an option to make conc download use or not cookie, default value
will be false.
- In AsyncSampler make a Clone with current cookies of Parent
CookieManager (the one that is calling Executor) and store it in
 ThreadLocal
- Change HttpSamplerBase#getCookieManager to retrieve first in Thread
Local then in instance
- If option is true, when reading res in FutureHTTPSampleResult loop,
reinject cookies inside ParentCookie otherwise ignore them

 As I wrote earlier, I'm not sure it ever makes sense to handle cookies
 from embedded resources, so it would be simpler just to ignore them.

 I am not sure Frame couldn't be concerned by this case, so in my opinion it
should be a parameter not ignored by default.


 I'm looking at passing the current sampler to the AsyncSampler class,
 which can then clone it.
 I think that will create an independent set of cookies, so there can
 be no need to protect against concurrent updates.

Agree, that's how I imagined it, but then you agree you have to store
CookieManager in thread local ?


 We then need to consider if non-concurrent downloads should still be
 processed for cookies.


Conc and serial should have the same behaviour


PS : Does it mean that you are working on issue and will fix it on your side
?


  Regards
  Philippe
 
  On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 13:14, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   Sebb,
   Do you want me to provide a patch with ConcurrentHashMap where I will
  have
   to handle null keys and values (not same behaviour as HashMap) or we
  forget
   about this approach ?
 
  I don't think we have yet decided how best to handle concurrent
 downloads.
 
  e.g. should they even be setting cookies?
 
   Regards
   Philippe
  
   On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
  philippe.moua...@gmail.com
   wrote:
  
   Hello,
   Just a little update on my test.
   I added a clear and gc before each Map instanciation and results are
   different:
  
  - HashMapput:645
  - ConcurrentHashMapput:832
  - ConcurrentReaderHashMapput:620
  - HashMap get:17
  - ConcurrentHashMap get:32
  - ConcurrentReaderHashMap get:60
  
  
   So it kind of closes the debate, sorry for disturbance.
   Regards
   Philippe
  
  
  
  
   public class TestMap {
  
   public static void main(String[] args) {
   StopWatch timer = new StopWatch();
  
   Map map = new HashMap();
   testPut(timer, map, HashMap);
   timer.reset();
  
   map.clear();
   System.gc();
  
   map = new ConcurrentHashMap();
   testPut(timer, map, ConcurrentHashMap);
   timer.reset();
  
   map.clear();
   System.gc();
  
   map = new ConcurrentReaderHashMap();
   testPut(timer, map, ConcurrentReaderHashMap);
   timer.reset();
  
  
   map.clear();
   System.gc();
  
   map = new HashMap();
   testGet(timer, map, HashMap);
   timer.reset();
  
   map.clear();
   System.gc();
  
   map = new ConcurrentHashMap();
   testGet(timer, map, ConcurrentHashMap);
   timer.reset();
  
   map.clear();
   System.gc();
  
   map = new ConcurrentReaderHashMap();
   testGet(timer, map, ConcurrentReaderHashMap);
   timer.reset();
   }
  
   /**
* @param timer
*/
   private static void testGet(StopWatch timer, Map map, String
 type) {
   timer.start();
   for (int i = 0; i  100; i++) {
   map.get(i);
   }
   timer.stop();
   System.out.println(type+ get:+timer.getTime());
   }
  
   /**
* @param timer
*/
   private static void testPut(StopWatch timer, Map map, String
 type) {
   timer.start();
   for (int i = 0; i  100; i++) {
   map.put(i,i);
   }
   timer.stop();
   System.out.println(type+put:+timer.getTime());
   }
  
  
   Regards
   Philippe
  
  
   On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de
 wrote:
On 02.10.2011 23:17, Philippe Mouawad wrote:
Ok, hope we can do the same.
   
See https://issues.apache.org/jira/browse/XMLBEANS-447
   
We are not the only people, who doubt it's correct to include that
  class
   ...
   
There was also a discussion some time ago in another ASF project,
because the Sun license which is cited by Doug Lea has a no use
 in
nuclear facilities clause, which make it 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread sebb
On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com wrote:
 On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  You are right,
  Patch was just about quick fix before the more impacting fix.
 
  Here are my propositions regarding this more impacting fix:
 
    - Add an option to make conc download use or not cookie, default value
    will be false.
    - In AsyncSampler make a Clone with current cookies of Parent
    CookieManager (the one that is calling Executor) and store it in
 ThreadLocal
    - Change HttpSamplerBase#getCookieManager to retrieve first in Thread
    Local then in instance
    - If option is true, when reading res in FutureHTTPSampleResult loop,
    reinject cookies inside ParentCookie otherwise ignore them

 As I wrote earlier, I'm not sure it ever makes sense to handle cookies
 from embedded resources, so it would be simpler just to ignore them.

 I am not sure Frame couldn't be concerned by this case, so in my opinion it
 should be a parameter not ignored by default.

I'd overlooked frame.


 I'm looking at passing the current sampler to the AsyncSampler class,
 which can then clone it.
 I think that will create an independent set of cookies, so there can
 be no need to protect against concurrent updates.

 Agree, that's how I imagined it, but then you agree you have to store
 CookieManager in thread local ?

If cookies are being ignored, then the cookie manager property can
just be cleared - i.e. there is no cookie manager.

Alternatively, it will also need to be cloned.


 We then need to consider if non-concurrent downloads should still be
 processed for cookies.


 Conc and serial should have the same behaviour


 PS : Does it mean that you are working on issue and will fix it on your side

Not yet decided; we need to agree on the approach first.



  Regards
  Philippe
 
  On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 13:14, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   Sebb,
   Do you want me to provide a patch with ConcurrentHashMap where I will
  have
   to handle null keys and values (not same behaviour as HashMap) or we
  forget
   about this approach ?
 
  I don't think we have yet decided how best to handle concurrent
 downloads.
 
  e.g. should they even be setting cookies?
 
   Regards
   Philippe
  
   On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
  philippe.moua...@gmail.com
   wrote:
  
   Hello,
   Just a little update on my test.
   I added a clear and gc before each Map instanciation and results are
   different:
  
      - HashMapput:645
      - ConcurrentHashMapput:832
      - ConcurrentReaderHashMapput:620
      - HashMap get:17
      - ConcurrentHashMap get:32
      - ConcurrentReaderHashMap get:60
  
  
   So it kind of closes the debate, sorry for disturbance.
   Regards
   Philippe
  
  
  
  
   public class TestMap {
  
       public static void main(String[] args) {
           StopWatch timer = new StopWatch();
  
           Map map = new HashMap();
           testPut(timer, map, HashMap);
           timer.reset();
  
           map.clear();
           System.gc();
  
           map = new ConcurrentHashMap();
           testPut(timer, map, ConcurrentHashMap);
           timer.reset();
  
           map.clear();
           System.gc();
  
           map = new ConcurrentReaderHashMap();
           testPut(timer, map, ConcurrentReaderHashMap);
           timer.reset();
  
  
           map.clear();
           System.gc();
  
           map = new HashMap();
           testGet(timer, map, HashMap);
           timer.reset();
  
           map.clear();
           System.gc();
  
           map = new ConcurrentHashMap();
           testGet(timer, map, ConcurrentHashMap);
           timer.reset();
  
           map.clear();
           System.gc();
  
           map = new ConcurrentReaderHashMap();
           testGet(timer, map, ConcurrentReaderHashMap);
           timer.reset();
       }
  
       /**
        * @param timer
        */
       private static void testGet(StopWatch timer, Map map, String
 type) {
           timer.start();
           for (int i = 0; i  100; i++) {
               map.get(i);
           }
           timer.stop();
           System.out.println(type+ get:+timer.getTime());
       }
  
       /**
        * @param timer
        */
       private static void testPut(StopWatch timer, Map map, String
 type) {
           timer.start();
           for (int i = 0; i  100; i++) {
               map.put(i,i);
           }
           timer.stop();
           System.out.println(type+put:+timer.getTime());
       }
  
  
   Regards
   Philippe
  
  
   On Mon, Oct 3, 2011 at 1:37 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 12:15, Rainer Jung rainer.j...@kippdata.de
 wrote:
On 02.10.2011 23:17, Philippe Mouawad wrote:
Ok, hope we can do the same.
   
See 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Shmuel Krakower
Hi,
Why isn't Cookie manager implemented the way Cache manager is (using a
thread local hash map)?

If it would be implemented the same way - the fix should be the same as on
the Cache manager case (
https://issues.apache.org/bugzilla/show_bug.cgi?id=51752)

Regards,
Shmuel Krakower.


On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   You are right,
   Patch was just about quick fix before the more impacting fix.
  
   Here are my propositions regarding this more impacting fix:
  
 - Add an option to make conc download use or not cookie, default
 value
 will be false.
 - In AsyncSampler make a Clone with current cookies of Parent
 CookieManager (the one that is calling Executor) and store it in
  ThreadLocal
 - Change HttpSamplerBase#getCookieManager to retrieve first in
 Thread
 Local then in instance
 - If option is true, when reading res in FutureHTTPSampleResult
 loop,
 reinject cookies inside ParentCookie otherwise ignore them
 
  As I wrote earlier, I'm not sure it ever makes sense to handle cookies
  from embedded resources, so it would be simpler just to ignore them.
 
  I am not sure Frame couldn't be concerned by this case, so in my opinion
 it
  should be a parameter not ignored by default.

 I'd overlooked frame.

 
  I'm looking at passing the current sampler to the AsyncSampler class,
  which can then clone it.
  I think that will create an independent set of cookies, so there can
  be no need to protect against concurrent updates.
 
  Agree, that's how I imagined it, but then you agree you have to store
  CookieManager in thread local ?

 If cookies are being ignored, then the cookie manager property can
 just be cleared - i.e. there is no cookie manager.

 Alternatively, it will also need to be cloned.

 
  We then need to consider if non-concurrent downloads should still be
  processed for cookies.
 
 
  Conc and serial should have the same behaviour

 
  PS : Does it mean that you are working on issue and will fix it on your
 side

 Not yet decided; we need to agree on the approach first.

 
 
   Regards
   Philippe
  
   On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 13:14, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
Sebb,
Do you want me to provide a patch with ConcurrentHashMap where I
 will
   have
to handle null keys and values (not same behaviour as HashMap) or
 we
   forget
about this approach ?
  
   I don't think we have yet decided how best to handle concurrent
  downloads.
  
   e.g. should they even be setting cookies?
  
Regards
Philippe
   
On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
   philippe.moua...@gmail.com
wrote:
   
Hello,
Just a little update on my test.
I added a clear and gc before each Map instanciation and results
 are
different:
   
   - HashMapput:645
   - ConcurrentHashMapput:832
   - ConcurrentReaderHashMapput:620
   - HashMap get:17
   - ConcurrentHashMap get:32
   - ConcurrentReaderHashMap get:60
   
   
So it kind of closes the debate, sorry for disturbance.
Regards
Philippe
   
   
   
   
public class TestMap {
   
public static void main(String[] args) {
StopWatch timer = new StopWatch();
   
Map map = new HashMap();
testPut(timer, map, HashMap);
timer.reset();
   
map.clear();
System.gc();
   
map = new ConcurrentHashMap();
testPut(timer, map, ConcurrentHashMap);
timer.reset();
   
map.clear();
System.gc();
   
map = new ConcurrentReaderHashMap();
testPut(timer, map, ConcurrentReaderHashMap);
timer.reset();
   
   
map.clear();
System.gc();
   
map = new HashMap();
testGet(timer, map, HashMap);
timer.reset();
   
map.clear();
System.gc();
   
map = new ConcurrentHashMap();
testGet(timer, map, ConcurrentHashMap);
timer.reset();
   
map.clear();
System.gc();
   
map = new ConcurrentReaderHashMap();
testGet(timer, map, ConcurrentReaderHashMap);
timer.reset();
}
   
/**
 * @param timer
 */
private static void testGet(StopWatch timer, Map map, String
  type) {
timer.start();
for (int i = 0; i  100; i++) {
map.get(i);
}
timer.stop();
System.out.println(type+ get:+timer.getTime());
}
   
/**
 * @param timer
 */
private static void 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread sebb
On 3 October 2011 16:07, Shmuel Krakower shmul...@gmail.com wrote:
 Hi,
 Why isn't Cookie manager implemented the way Cache manager is (using a
 thread local hash map)?

Probably mainly historical.

Cookies are currently stored in an ArrayList, but one could perhaps
use a HashMap instead.
Though that won't work if cookie names can change - the current
implementation does allow pre-defined cookies to be defined in terms
of variables/functions.

Is there a use-case for variable cookie names? Perhaps; it seems quite
likely that variable values would be useful.

I don't know off-hand if that would still work if the code switched to
(Inheritable)ThreadLocal.

 If it would be implemented the same way - the fix should be the same as on
 the Cache manager case (
 https://issues.apache.org/bugzilla/show_bug.cgi?id=51752)

 Regards,
 Shmuel Krakower.


 On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   You are right,
   Patch was just about quick fix before the more impacting fix.
  
   Here are my propositions regarding this more impacting fix:
  
     - Add an option to make conc download use or not cookie, default
 value
     will be false.
     - In AsyncSampler make a Clone with current cookies of Parent
     CookieManager (the one that is calling Executor) and store it in
  ThreadLocal
     - Change HttpSamplerBase#getCookieManager to retrieve first in
 Thread
     Local then in instance
     - If option is true, when reading res in FutureHTTPSampleResult
 loop,
     reinject cookies inside ParentCookie otherwise ignore them
 
  As I wrote earlier, I'm not sure it ever makes sense to handle cookies
  from embedded resources, so it would be simpler just to ignore them.
 
  I am not sure Frame couldn't be concerned by this case, so in my opinion
 it
  should be a parameter not ignored by default.

 I'd overlooked frame.

 
  I'm looking at passing the current sampler to the AsyncSampler class,
  which can then clone it.
  I think that will create an independent set of cookies, so there can
  be no need to protect against concurrent updates.
 
  Agree, that's how I imagined it, but then you agree you have to store
  CookieManager in thread local ?

 If cookies are being ignored, then the cookie manager property can
 just be cleared - i.e. there is no cookie manager.

 Alternatively, it will also need to be cloned.

 
  We then need to consider if non-concurrent downloads should still be
  processed for cookies.
 
 
  Conc and serial should have the same behaviour

 
  PS : Does it mean that you are working on issue and will fix it on your
 side

 Not yet decided; we need to agree on the approach first.

 
 
   Regards
   Philippe
  
   On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 13:14, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
Sebb,
Do you want me to provide a patch with ConcurrentHashMap where I
 will
   have
to handle null keys and values (not same behaviour as HashMap) or
 we
   forget
about this approach ?
  
   I don't think we have yet decided how best to handle concurrent
  downloads.
  
   e.g. should they even be setting cookies?
  
Regards
Philippe
   
On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
   philippe.moua...@gmail.com
wrote:
   
Hello,
Just a little update on my test.
I added a clear and gc before each Map instanciation and results
 are
different:
   
   - HashMapput:645
   - ConcurrentHashMapput:832
   - ConcurrentReaderHashMapput:620
   - HashMap get:17
   - ConcurrentHashMap get:32
   - ConcurrentReaderHashMap get:60
   
   
So it kind of closes the debate, sorry for disturbance.
Regards
Philippe
   
   
   
   
public class TestMap {
   
    public static void main(String[] args) {
        StopWatch timer = new StopWatch();
   
        Map map = new HashMap();
        testPut(timer, map, HashMap);
        timer.reset();
   
        map.clear();
        System.gc();
   
        map = new ConcurrentHashMap();
        testPut(timer, map, ConcurrentHashMap);
        timer.reset();
   
        map.clear();
        System.gc();
   
        map = new ConcurrentReaderHashMap();
        testPut(timer, map, ConcurrentReaderHashMap);
        timer.reset();
   
   
        map.clear();
        System.gc();
   
        map = new HashMap();
        testGet(timer, map, HashMap);
        timer.reset();
   
        map.clear();
        System.gc();
   
        map = new ConcurrentHashMap();
        testGet(timer, map, ConcurrentHashMap);
        timer.reset();
   
        map.clear();
        System.gc();
   
        

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Philippe Mouawad
Hello,
 By the way I think there is also an issue in CacheManager due to the use of
InheritableThreadLocal (which is not thread safe).
Indeed this Map is shared by concurrent children thread, so Map is accessed
concurrently but not thread-safe = ISSUE
I made a little main sample that I can attach to an issue if you want to
see Map is shared

See this:

   - http://www.ibm.com/developerworks/java/library/j-threads3/index.html


Regards
Philippe Mouawad

On Mon, Oct 3, 2011 at 5:28 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 16:07, Shmuel Krakower shmul...@gmail.com wrote:
  Hi,
  Why isn't Cookie manager implemented the way Cache manager is (using a
  thread local hash map)?

 Probably mainly historical.

 Cookies are currently stored in an ArrayList, but one could perhaps
 use a HashMap instead.
 Though that won't work if cookie names can change - the current
 implementation does allow pre-defined cookies to be defined in terms
 of variables/functions.

 Is there a use-case for variable cookie names? Perhaps; it seems quite
 likely that variable values would be useful.

 I don't know off-hand if that would still work if the code switched to
 (Inheritable)ThreadLocal.

  If it would be implemented the same way - the fix should be the same as
 on
  the Cache manager case (
  https://issues.apache.org/bugzilla/show_bug.cgi?id=51752)
 
  Regards,
  Shmuel Krakower.
 
 
  On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 14:04, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
You are right,
Patch was just about quick fix before the more impacting fix.
   
Here are my propositions regarding this more impacting fix:
   
  - Add an option to make conc download use or not cookie, default
  value
  will be false.
  - In AsyncSampler make a Clone with current cookies of Parent
  CookieManager (the one that is calling Executor) and store it in
   ThreadLocal
  - Change HttpSamplerBase#getCookieManager to retrieve first in
  Thread
  Local then in instance
  - If option is true, when reading res in FutureHTTPSampleResult
  loop,
  reinject cookies inside ParentCookie otherwise ignore them
  
   As I wrote earlier, I'm not sure it ever makes sense to handle
 cookies
   from embedded resources, so it would be simpler just to ignore them.
  
   I am not sure Frame couldn't be concerned by this case, so in my
 opinion
  it
   should be a parameter not ignored by default.
 
  I'd overlooked frame.
 
  
   I'm looking at passing the current sampler to the AsyncSampler class,
   which can then clone it.
   I think that will create an independent set of cookies, so there can
   be no need to protect against concurrent updates.
  
   Agree, that's how I imagined it, but then you agree you have to store
   CookieManager in thread local ?
 
  If cookies are being ignored, then the cookie manager property can
  just be cleared - i.e. there is no cookie manager.
 
  Alternatively, it will also need to be cloned.
 
  
   We then need to consider if non-concurrent downloads should still be
   processed for cookies.
  
  
   Conc and serial should have the same behaviour
 
  
   PS : Does it mean that you are working on issue and will fix it on
 your
  side
 
  Not yet decided; we need to agree on the approach first.
 
  
  
Regards
Philippe
   
On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
   
On 3 October 2011 13:14, Philippe Mouawad 
  philippe.moua...@gmail.com
wrote:
 Sebb,
 Do you want me to provide a patch with ConcurrentHashMap where I
  will
have
 to handle null keys and values (not same behaviour as HashMap)
 or
  we
forget
 about this approach ?
   
I don't think we have yet decided how best to handle concurrent
   downloads.
   
e.g. should they even be setting cookies?
   
 Regards
 Philippe

 On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
philippe.moua...@gmail.com
 wrote:

 Hello,
 Just a little update on my test.
 I added a clear and gc before each Map instanciation and
 results
  are
 different:

- HashMapput:645
- ConcurrentHashMapput:832
- ConcurrentReaderHashMapput:620
- HashMap get:17
- ConcurrentHashMap get:32
- ConcurrentReaderHashMap get:60


 So it kind of closes the debate, sorry for disturbance.
 Regards
 Philippe




 public class TestMap {

 public static void main(String[] args) {
 StopWatch timer = new StopWatch();

 Map map = new HashMap();
 testPut(timer, map, HashMap);
 timer.reset();

 map.clear();
 System.gc();

 map = new ConcurrentHashMap();
   

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-03 Thread Philippe Mouawad
I opened :
https://issues.apache.org/bugzilla/show_bug.cgi?id=51942
and provided patch.

Regards
Philippe

On Mon, Oct 3, 2011 at 6:16 PM, Philippe Mouawad philippe.moua...@gmail.com
 wrote:

 Hello,
  By the way I think there is also an issue in CacheManager due to the use
 of InheritableThreadLocal (which is not thread safe).
 Indeed this Map is shared by concurrent children thread, so Map is accessed
 concurrently but not thread-safe = ISSUE
 I made a little main sample that I can attach to an issue if you want to
 see Map is shared

 See this:

- http://www.ibm.com/developerworks/java/library/j-threads3/index.html


 Regards
 Philippe Mouawad


 On Mon, Oct 3, 2011 at 5:28 PM, sebb seb...@gmail.com wrote:

 On 3 October 2011 16:07, Shmuel Krakower shmul...@gmail.com wrote:
  Hi,
  Why isn't Cookie manager implemented the way Cache manager is (using a
  thread local hash map)?

 Probably mainly historical.

 Cookies are currently stored in an ArrayList, but one could perhaps
 use a HashMap instead.
 Though that won't work if cookie names can change - the current
 implementation does allow pre-defined cookies to be defined in terms
 of variables/functions.

 Is there a use-case for variable cookie names? Perhaps; it seems quite
 likely that variable values would be useful.

 I don't know off-hand if that would still work if the code switched to
 (Inheritable)ThreadLocal.

  If it would be implemented the same way - the fix should be the same as
 on
  the Cache manager case (
  https://issues.apache.org/bugzilla/show_bug.cgi?id=51752)
 
  Regards,
  Shmuel Krakower.
 
 
  On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:
 
  On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
  wrote:
   On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
  
   On 3 October 2011 14:04, Philippe Mouawad 
 philippe.moua...@gmail.com
   wrote:
You are right,
Patch was just about quick fix before the more impacting fix.
   
Here are my propositions regarding this more impacting fix:
   
  - Add an option to make conc download use or not cookie, default
  value
  will be false.
  - In AsyncSampler make a Clone with current cookies of Parent
  CookieManager (the one that is calling Executor) and store it in
   ThreadLocal
  - Change HttpSamplerBase#getCookieManager to retrieve first in
  Thread
  Local then in instance
  - If option is true, when reading res in
 FutureHTTPSampleResult
  loop,
  reinject cookies inside ParentCookie otherwise ignore them
  
   As I wrote earlier, I'm not sure it ever makes sense to handle
 cookies
   from embedded resources, so it would be simpler just to ignore them.
  
   I am not sure Frame couldn't be concerned by this case, so in my
 opinion
  it
   should be a parameter not ignored by default.
 
  I'd overlooked frame.
 
  
   I'm looking at passing the current sampler to the AsyncSampler
 class,
   which can then clone it.
   I think that will create an independent set of cookies, so there can
   be no need to protect against concurrent updates.
  
   Agree, that's how I imagined it, but then you agree you have to store
   CookieManager in thread local ?
 
  If cookies are being ignored, then the cookie manager property can
  just be cleared - i.e. there is no cookie manager.
 
  Alternatively, it will also need to be cloned.
 
  
   We then need to consider if non-concurrent downloads should still be
   processed for cookies.
  
  
   Conc and serial should have the same behaviour
 
  
   PS : Does it mean that you are working on issue and will fix it on
 your
  side
 
  Not yet decided; we need to agree on the approach first.
 
  
  
Regards
Philippe
   
On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
   
On 3 October 2011 13:14, Philippe Mouawad 
  philippe.moua...@gmail.com
wrote:
 Sebb,
 Do you want me to provide a patch with ConcurrentHashMap where
 I
  will
have
 to handle null keys and values (not same behaviour as HashMap)
 or
  we
forget
 about this approach ?
   
I don't think we have yet decided how best to handle concurrent
   downloads.
   
e.g. should they even be setting cookies?
   
 Regards
 Philippe

 On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad 
philippe.moua...@gmail.com
 wrote:

 Hello,
 Just a little update on my test.
 I added a clear and gc before each Map instanciation and
 results
  are
 different:

- HashMapput:645
- ConcurrentHashMapput:832
- ConcurrentReaderHashMapput:620
- HashMap get:17
- ConcurrentHashMap get:32
- ConcurrentReaderHashMap get:60


 So it kind of closes the debate, sorry for disturbance.
 Regards
 Philippe




 public class TestMap {

 public static void main(String[] args) {
 StopWatch timer = new StopWatch();

 Map 

Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-02 Thread sebb
On 1 October 2011 14:33, Philippe Mouawad philippe.moua...@gmail.com wrote:
 A little additional note,
 There is an implementation of Concurrent map by doug lea in concurrent.jar
 called ConcurrentReaderHashMap
 that has same performance as HashMap in read and a little less on write.
 But performances are much better than ConcurrentHashMap.
 So maybe a better alternative but requires concurrent-1.3.4.jar from:

   -
   http://g.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/concurrent/intro.html


That looks interesting, however the licensing is not at all clear to
me, and AFAICT we would only want the one class, not the entire jar.

So it's quite a lot of extra complication for what is just a local optimisation.

-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-02 Thread sebb
On 2 October 2011 15:20, Rainer Jung rainer.j...@kippdata.de wrote:
 On 02.10.2011 15:49, sebb wrote:
 On 1 October 2011 14:33, Philippe Mouawad philippe.moua...@gmail.com wrote:
 A little additional note,
 There is an implementation of Concurrent map by doug lea in concurrent.jar
 called ConcurrentReaderHashMap
 that has same performance as HashMap in read and a little less on write.
 But performances are much better than ConcurrentHashMap.
 So maybe a better alternative but requires concurrent-1.3.4.jar from:

   -
   http://g.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/concurrent/intro.html


 That looks interesting, however the licensing is not at all clear to
 me, and AFAICT we would only want the one class, not the entire jar.

 So it's quite a lot of extra complication for what is just a local 
 optimisation.

 Concerning license the page says:

 All classes are released to the public domain and may be used for any
 purpose whatsoever without permission or acknowledgment. Portions of the
 CopyOnWriteArrayList and ConcurrentReaderHashMap classes are adapted
 from Sun JDK source code. These are copyright of Sun Microsystems, Inc,
 and are used with their kind permission, as described in this license.

 and this license is another link to a PDF.

 Legal has already resolved that one:

 http://www.apache.org/legal/resolved.html#concurrent

Thanks! Very useful.

That would allow us to use the class as part of the concurrent library.
However, we cannot use that particular source file, because it is one
of the Sun-licensed ones.
Not sure if there is any other way to include just the class that we need.

-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-02 Thread Philippe Mouawad
Hello Sebb,
XMLBeans which is an Apache project under Apache Licence has included it :

   -
   
http://massapi.com/source/xmlbeans-2.5.0/src/common/org/apache/xmlbeans/impl/common/ConcurrentReaderHashMap.java.html


Couldn't we do the same ?
Regards
Philippe
On Sun, Oct 2, 2011 at 4:29 PM, sebb seb...@gmail.com wrote:

 On 2 October 2011 15:20, Rainer Jung rainer.j...@kippdata.de wrote:
  On 02.10.2011 15:49, sebb wrote:
  On 1 October 2011 14:33, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  A little additional note,
  There is an implementation of Concurrent map by doug lea in
 concurrent.jar
  called ConcurrentReaderHashMap
  that has same performance as HashMap in read and a little less on
 write.
  But performances are much better than ConcurrentHashMap.
  So maybe a better alternative but requires concurrent-1.3.4.jar from:
 
-
 
 http://g.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/concurrent/intro.html
 
 
  That looks interesting, however the licensing is not at all clear to
  me, and AFAICT we would only want the one class, not the entire jar.
 
  So it's quite a lot of extra complication for what is just a local
 optimisation.
 
  Concerning license the page says:
 
  All classes are released to the public domain and may be used for any
  purpose whatsoever without permission or acknowledgment. Portions of the
  CopyOnWriteArrayList and ConcurrentReaderHashMap classes are adapted
  from Sun JDK source code. These are copyright of Sun Microsystems, Inc,
  and are used with their kind permission, as described in this license.
 
  and this license is another link to a PDF.
 
  Legal has already resolved that one:
 
  http://www.apache.org/legal/resolved.html#concurrent

 Thanks! Very useful.

 That would allow us to use the class as part of the concurrent library.
 However, we cannot use that particular source file, because it is one
 of the Sun-licensed ones.
 Not sure if there is any other way to include just the class that we need.

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




-- 
Cordialement.
Philippe Mouawad.


Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-02 Thread sebb
On 2 October 2011 19:39, Philippe Mouawad philippe.moua...@gmail.com wrote:
 Hello Sebb,
 XMLBeans which is an Apache project under Apache Licence has included it :

   -
   
 http://massapi.com/source/xmlbeans-2.5.0/src/common/org/apache/xmlbeans/impl/common/ConcurrentReaderHashMap.java.html


 Couldn't we do the same ?

Possibly; but we would need to be sure that their reading of the rules
was correct; we don't want to propagate a mistake if it is one.

I'm pretty sure that they should not have added the AL header to the
file, so I wonder if they should have included the file at all.

-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-02 Thread Philippe Mouawad
Ok, hope we can do the same.
I must say I don't understand why ConcurrentReaderHashMap is not in JDK.

Regards
Philippe

On Sun, Oct 2, 2011 at 11:07 PM, sebb seb...@gmail.com wrote:

 On 2 October 2011 19:39, Philippe Mouawad philippe.moua...@gmail.com
 wrote:
  Hello Sebb,
  XMLBeans which is an Apache project under Apache Licence has included it
 :
 
-
 
 http://massapi.com/source/xmlbeans-2.5.0/src/common/org/apache/xmlbeans/impl/common/ConcurrentReaderHashMap.java.html
 
 
  Couldn't we do the same ?

 Possibly; but we would need to be sure that their reading of the rules
 was correct; we don't want to propagate a mistake if it is one.

 I'm pretty sure that they should not have added the AL header to the
 file, so I wonder if they should have included the file at all.

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




-- 
Cordialement.
Philippe Mouawad.


Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-01 Thread sebb
On 1 October 2011 10:53, Philippe Mouawad p.moua...@ubik-ingenierie.com wrote:
 Hello Milamber, Sebb, All,
 Regarding 51919 https://issues.apache.org/bugzilla/show_bug.cgi?id=51919,
 I wonder if there is not an issue in JMeterVariables access introduced by
 concurrent download.
 Initially I think JMeterVariables were not supposed to be shared so object
 not thread safe, today they are due to Conc download.
 Maybe JMeterVariables#variables should be replaced by a ConcurrentHashMap.
 If so CONTEXT_VARIABLES_LOCK should be removed in my patch.
 What do you think?

Yes, a lot of JMeter code assumes that samplers and thread variables
are not shared.

So either we remove those assumptions, which might prove more
expensive in the non-concurrent case; or we change the way the
concurrent downloads are handled so they don't directly access the
main thread variables.

One way might be to clone the sampler so it effectively becomes a new
JMeterThread; I don't know how easy that would be, we would also need
to recover the updates at the end of the sub-samples.

Another way would be to intercept the writes to the objects that are
not thread-safe and store them up for the main sample thread to do at
the end.

Either way, at present the concurrent downloads unfortunately have
problems with cookie handling (and with buffer handling, but that is
trivial to fix).

So a short-term approach might be to ignore cookies when performing
sub-samples - I think it is only the cookies that require updates to
the thread variables.

Are there any use-cases where the web application relies on cookies
that are set by resources embedded in the main page?
I know some sites set cookies on embedded resources, but is that
necessary, or is it a by-product?

If not, then ignoring such cookies would be a long-term solution.

 Regards
 Philippe


 On Sat, Oct 1, 2011 at 9:34 AM, bugzi...@apache.org wrote:

 https://issues.apache.org/bugzilla/show_bug.cgi?id=51919

 --- Comment #2 from Milamber milam...@apache.org 2011-10-01 07:34:04 UTC
 ---
 A better way is to synchronize only the code section referred to the
 variables
 from JMeterContext
 (in add() and removeMatchingCookies() methods, I thinks)

 --
 Configure bugmail:
 https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
 --- You are receiving this mail because: ---
 You are on the CC list for the bug.
 You reported the bug.




 --
 Cordialement.
 Philippe Mouawad.
 Ubik-Ingénierie


-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org



Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-01 Thread Philippe Mouawad
On Sat, Oct 1, 2011 at 12:57 PM, sebb seb...@gmail.com wrote:

 On 1 October 2011 10:53, Philippe Mouawad p.moua...@ubik-ingenierie.com
 wrote:
  Hello Milamber, Sebb, All,
  Regarding 51919 
 https://issues.apache.org/bugzilla/show_bug.cgi?id=51919,
  I wonder if there is not an issue in JMeterVariables access introduced by
  concurrent download.
  Initially I think JMeterVariables were not supposed to be shared so
 object
  not thread safe, today they are due to Conc download.
  Maybe JMeterVariables#variables should be replaced by a
 ConcurrentHashMap.
  If so CONTEXT_VARIABLES_LOCK should be removed in my patch.
  What do you think?

 Yes, a lot of JMeter code assumes that samplers and thread variables
 are not shared.

 So either we remove those assumptions, which might prove more
 expensive in the non-concurrent case;


= Performance impact is around 3 times more between ConcurrentHashMap and
HashMap
when only one thread is using Map (ie case when no concurrent downloads
occur) but maybe
it is not that important regarding other parts of code, needs some study.


 or we change the way the
 concurrent downloads are handled so they don't directly access the
 main thread variables.

 = Don't you think code will be hard to maintain ? today AsyncSampler uses
same sample() method as others
won't there be lot of special cases ?


 One way might be to clone the sampler so it effectively becomes a new
 JMeterThread; I don't know how easy that would be, we would also need
 to recover the updates at the end of the sub-samples.

 Another way would be to intercept the writes to the objects that are
 not thread-safe and store them up for the main sample thread to do at
 the end.

 Either way, at present the concurrent downloads unfortunately have
 problems with cookie handling (and with buffer handling, but that is
 trivial to fix).

 So a short-term approach might be to ignore cookies when performing
 sub-samples - I think it is only the cookies that require updates to
 the thread variables.

 Are there any use-cases where the web application relies on cookies
 that are set by resources embedded in the main page?
 I know some sites set cookies on embedded resources, but is that
 necessary, or is it a by-product?


= I agree, I have never met this case in my load tests but if it is met
load testing application will be hard


 If not, then ignoring such cookies would be a long-term solution.

  Regards
  Philippe
 
 
  On Sat, Oct 1, 2011 at 9:34 AM, bugzi...@apache.org wrote:
 
  https://issues.apache.org/bugzilla/show_bug.cgi?id=51919
 
  --- Comment #2 from Milamber milam...@apache.org 2011-10-01 07:34:04
 UTC
  ---
  A better way is to synchronize only the code section referred to the
  variables
  from JMeterContext
  (in add() and removeMatchingCookies() methods, I thinks)
 
  --
  Configure bugmail:
  https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
  --- You are receiving this mail because: ---
  You are on the CC list for the bug.
  You reported the bug.
 
 
 
 
  --
  Cordialement.
  Philippe Mouawad.
  Ubik-Ingénierie
 

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




-- 
Cordialement.
Philippe Mouawad.


Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download

2011-10-01 Thread sebb
On 1 October 2011 12:38, Philippe Mouawad philippe.moua...@gmail.com wrote:
 On Sat, Oct 1, 2011 at 12:57 PM, sebb seb...@gmail.com wrote:

 On 1 October 2011 10:53, Philippe Mouawad p.moua...@ubik-ingenierie.com
 wrote:
  Hello Milamber, Sebb, All,
  Regarding 51919 
 https://issues.apache.org/bugzilla/show_bug.cgi?id=51919,
  I wonder if there is not an issue in JMeterVariables access introduced by
  concurrent download.
  Initially I think JMeterVariables were not supposed to be shared so
 object
  not thread safe, today they are due to Conc download.
  Maybe JMeterVariables#variables should be replaced by a
 ConcurrentHashMap.
  If so CONTEXT_VARIABLES_LOCK should be removed in my patch.
  What do you think?

 Yes, a lot of JMeter code assumes that samplers and thread variables
 are not shared.

 So either we remove those assumptions, which might prove more
 expensive in the non-concurrent case;


 = Performance impact is around 3 times more between ConcurrentHashMap and
 HashMap
 when only one thread is using Map (ie case when no concurrent downloads
 occur) but maybe
 it is not that important regarding other parts of code, needs some study.

Indeed, study is needed.


 or we change the way the
 concurrent downloads are handled so they don't directly access the
 main thread variables.

 = Don't you think code will be hard to maintain ? today AsyncSampler uses
 same sample() method as others

That is presumably broken as well, then.

 won't there be lot of special cases ?


Potentially yes, but the alternative is to revisit and potentially
rewrite large chunks of JMeter code.

That needs a proper strategy first, as well as loads of tests to check
the behaviour.

 One way might be to clone the sampler so it effectively becomes a new
 JMeterThread; I don't know how easy that would be, we would also need
 to recover the updates at the end of the sub-samples.

 Another way would be to intercept the writes to the objects that are
 not thread-safe and store them up for the main sample thread to do at
 the end.

 Either way, at present the concurrent downloads unfortunately have
 problems with cookie handling (and with buffer handling, but that is
 trivial to fix).

 So a short-term approach might be to ignore cookies when performing
 sub-samples - I think it is only the cookies that require updates to
 the thread variables.

 Are there any use-cases where the web application relies on cookies
 that are set by resources embedded in the main page?
 I know some sites set cookies on embedded resources, but is that
 necessary, or is it a by-product?


 = I agree, I have never met this case in my load tests but if it is met
 load testing application will be hard


 If not, then ignoring such cookies would be a long-term solution.

  Regards
  Philippe
 
 
  On Sat, Oct 1, 2011 at 9:34 AM, bugzi...@apache.org wrote:
 
  https://issues.apache.org/bugzilla/show_bug.cgi?id=51919
 
  --- Comment #2 from Milamber milam...@apache.org 2011-10-01 07:34:04
 UTC
  ---
  A better way is to synchronize only the code section referred to the
  variables
  from JMeterContext
  (in add() and removeMatchingCookies() methods, I thinks)
 
  --
  Configure bugmail:
  https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
  --- You are receiving this mail because: ---
  You are on the CC list for the bug.
  You reported the bug.
 
 
 
 
  --
  Cordialement.
  Philippe Mouawad.
  Ubik-Ingénierie
 

 -
 To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
 For additional commands, e-mail: dev-h...@jakarta.apache.org




 --
 Cordialement.
 Philippe Mouawad.


-
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org