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