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 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.

Reply via email to