Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
