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 Future<HTTPSampleResult> > >> 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 < 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 <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 > >> >> >> > >> >> >> > >> >> > > >> >> > > >> >> > -- > >> >> > Cordialement. > >> >> > Philippe Mouawad. > >> >> > > >> >> > >> >> --------------------------------------------------------------------- > >> >> 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 > >> > >> > > > > > > -- > > Cordialement. > > Philippe Mouawad. > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org > For additional commands, e-mail: dev-h...@jakarta.apache.org > > -- Cordialement. Philippe Mouawad.