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