Return-Path: X-Original-To: apmail-jakarta-dev-archive@minotaur.apache.org Delivered-To: apmail-jakarta-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1817B9358 for ; Mon, 3 Oct 2011 14:40:17 +0000 (UTC) Received: (qmail 25814 invoked by uid 500); 3 Oct 2011 14:40:16 -0000 Delivered-To: apmail-jakarta-dev-archive@jakarta.apache.org Received: (qmail 25656 invoked by uid 500); 3 Oct 2011 14:40:16 -0000 Mailing-List: contact dev-help@jakarta.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jakarta.apache.org Delivered-To: mailing list dev@jakarta.apache.org Received: (qmail 25648 invoked by uid 99); 3 Oct 2011 14:40:16 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 03 Oct 2011 14:40:16 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=FREEMAIL_FROM,HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of philippe.mouawad@gmail.com designates 209.85.213.44 as permitted sender) Received: from [209.85.213.44] (HELO mail-yw0-f44.google.com) (209.85.213.44) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 03 Oct 2011 14:40:11 +0000 Received: by ywm3 with SMTP id 3so1138346ywm.31 for ; Mon, 03 Oct 2011 07:39:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=bINMkpPtaQnFvFefdVM03EEvsrHdnCVJOSTjs8k6Lys=; b=C58G0x2YwrDfXpMhLtS7zjef43gAizVK4OV5ulIPFvAKpOxUKMKDr2nEK8WBiraYVp 8xx2wCbkrB2Bq2YpF8Q9S26bfq6YUCPVlNgqMaa5uQBFygCfRYC+IIp7YI67dvo2JC0U hoEMWE3DUlN+ySzMxoX7XnIG6X6qLyCTRjg5U= MIME-Version: 1.0 Received: by 10.236.35.205 with SMTP id u53mr13339702yha.120.1317652790657; Mon, 03 Oct 2011 07:39:50 -0700 (PDT) Received: by 10.236.208.228 with HTTP; Mon, 3 Oct 2011 07:39:50 -0700 (PDT) In-Reply-To: References: <4E887341.9050506@kippdata.de> <4E899942.3010503@kippdata.de> Date: Mon, 3 Oct 2011 16:39:50 +0200 Message-ID: Subject: Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download From: Philippe Mouawad To: dev@jakarta.apache.org Content-Type: multipart/alternative; boundary=20cf301afd6186413c04ae65f296 --20cf301afd6186413c04ae65f296 Content-Type: text/plain; charset=ISO-8859-1 On Mon, Oct 3, 2011 at 4:31 PM, sebb wrote: > On 3 October 2011 14:04, Philippe Mouawad > 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 Future 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 wrote: > > > >> On 3 October 2011 13:14, Philippe Mouawad > >> 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.mouawad@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 < 1000000; 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 < 1000000; 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 wrote: > >> >> > >> >>> On 3 October 2011 12:15, Rainer Jung > 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-unsubscribe@jakarta.apache.org > >> >>> > For additional commands, e-mail: dev-help@jakarta.apache.org > >> >>> > > >> >>> > > >> >>> > >> >>> > --------------------------------------------------------------------- > >> >>> To unsubscribe, e-mail: dev-unsubscribe@jakarta.apache.org > >> >>> For additional commands, e-mail: dev-help@jakarta.apache.org > >> >>> > >> >>> > >> >> > >> >> > >> >> -- > >> >> Cordialement. > >> >> Philippe Mouawad. > >> >> > >> >> > >> >> > >> >> > >> > > >> > > >> > -- > >> > Cordialement. > >> > Philippe Mouawad. > >> > > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscribe@jakarta.apache.org > >> For additional commands, e-mail: dev-help@jakarta.apache.org > >> > >> > > > > > > -- > > Cordialement. > > Philippe Mouawad. > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscribe@jakarta.apache.org > For additional commands, e-mail: dev-help@jakarta.apache.org > > -- Cordialement. Philippe Mouawad. --20cf301afd6186413c04ae65f296--