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.
Regarding the need to clone sampler, why do we need this, is there some data
that may be corrupted except CookieManager ?
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.
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.
