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

Reply via email to