On 7 October 2011 09:45, Philippe Mouawad <philippe.moua...@gmail.com> wrote:
> Hello Sebb,
> By proxy do you mean:
> Proxy.newProxyInstance and InvocationHandler, if so aren't we missing an
> Interface (Sampler does not do the job and it does not use Entry parameter).
> If so I agree lot of job.
Yes.
> Regarding the need to clone sampler, why do we need this, is there some data
> that may be corrupted except CookieManager ?
I don't know.
We already discovered problems with CM and CacheManager, but there are
a lot of other data areas that could potentially be affected.
For example, the HC3 and HC4 clients and their connections.
As I wrote earlier, the JMeter design is based on sampler classes
being single-threaded.
Just had a quick look, and for example the HC4 currentRequest (used
for interrupting the sampler) is a potential problem.
Any instance data that can be changed by the sampling process may
cause problems.
> I implemented the approach with CookieManager in ThreadLocal and it works
> without need for thread safety.
> I will attach it to ISSUE to let you review it.
OK, I'll have a look.
However, as I just wrote, I suspect this will only fix the issue we know about.
> Regards
> Philippe
>
> On Fri, Oct 7, 2011 at 2:46 AM, sebb <seb...@gmail.com> wrote:
>
>> On 7 October 2011 00:00, sebb <seb...@gmail.com> wrote:
>> > 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.
>>
>> Unfortunately, cloning the sampler is not easy.
>> I've had a look at wrapping the sampler to intercept calls to
>> getCookieManager() but AFAICT that would require writing an
>> interceptor proxy.
>> Which is potentially a lot of work.
>>
>> I need to give this some more thought.
>>
>> >>
>> >>> > - 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
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org
For additional commands, e-mail: dev-h...@jakarta.apache.org
