jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: JSR223 elements performance
Date Sat, 29 Aug 2015 21:39:28 GMT
On Saturday, August 29, 2015, sebb <sebbaz@gmail.com> wrote:

> On 29 August 2015 at 20:53, Vladimir Sitnikov
> <sitnikov.vladimir@gmail.com <javascript:;>> wrote:
> >>> 2.1) What if we just "reuse global scope" between all the samplers in
> >>> the current thread? At the end, users should write proper code
> >>> (without global variables)
> >
> >>That might break existing test plans.
> >
> > Of course it can. What if we leave a jmeter.properties kind of
> > property to revert old behavior?

+1 for this idea


> > In fact, "global scope sharing/unsharing" is undefined behavior (I do
> > not find it documented), thus we can just tweak it to suit the
> > majority :)
>
> Not all the behaviour of JMeter is fully documented, but that does not
> mean we can just change it.
>
> true

> >>> 2.2) If "reuse global scope always" is considered a no-go solution,
> >>> should we add a UI checkbox to "enable/disable" sharing?
> >> Surely that's what the cache key already achieves?
> >
> > Unfortunately, it does not. Even with a "cache key", the simple loop
> > above is slow: it takes 1.2 seconds in nashorn javascript while
> > implementing global scope caching reduces that to 30ms.
>
> >>> Does it really make sense to ask user to specify cache key?
> >>
> >> Yes, because only the user knows if the scripts can be shared.
> >

If user uses ${} then using compilation would break his behaviour .
Making it a checkbox is better and clearer than cache key.


> I do not follow you. A script is not data. Sharing scripts should have
> > no visible impact except "memory consumption" for the cache.
>
> The scripts are mutable, because they can contain variable references.
>
> > "cache key" just enables compilation cache. The compilation itself
> > does not touch global scope or variables or whatever.
> > It just parses the script and creates "ready-to-eval" CompiledScript
> object.
>
> Exactly, so it's important that the script is immutable.
>
> > I believe, the best practice of writing JSR223 scripts (if writing at
> > all:) is to avoid ${...} interpolations in the script text (it is
> > documented).
>
> But including interpolation can be useful, and is available everywhere
> else.
> We should not break this feature arbitrarily.
>
>

> In other words, script text should be constant, so I do not see why
> > JMeter should refrain from caching scripts by default.
>
> Because that will break some scripts, and we try to make JMeter
> upwards compatible as far as is possible.
>
> We could make caching the default behaviour(check checkbox) and on
existing plans if cache key is not filled uncheck it.
It wouldn't break anything.


> >>> It might happen script itself defines some global variables or
> >>> redefines out of the box ones.
> >>
> >> That is why the cache key is manually provided.
> >
> > Frankly speaking, "cache key" looks like an implementation glitch.
> > 0) "cache key" is somewhat strange. It is rather understandable by a
> > programmer, but I think it is completely obscure for non-programmers.
> > It is not clear what are the consequences of non-null cache key.
>
> This just needs to be explained better then - patches welcome.
>
> > 0) Why cache key is a string? I think it should be just a boolean.
>
> Not sure why it was done this way, but it is done now, and changing it
> would break test plans.
>
>  Making it a text allows you to reuse the compiled object at other places
but it's a hack as you need to put at least 1 char in script (we could fix
it to allow clean reuse, ie if there is a cache key but script is empty ,
look for compiled cached script)

But when I implemented it I exposed a technical detail (the cache key) , I
shouldn't have done it
Reverting to boolean would be possible but would require us to manage
migration.


> 1) Global scope of 223 engine (e.g. javascript globals) is not related
> > to "cache key". It (global scope sharing) might require yet another
> > option that tells something like "reuse global scope 223". I do not
> > think there are cases when users would want having several different
> > CACHED global contexts.
>
> I don't know for sure that there is no use case where the same script
> needs to have different global contexts.
>
> > 2) "global scope issue" is not obvious from UI/documentation. The bad
> > thing is "default" and the only mode is to "spawn new global scope
> > every time", thus it impacts performance for no apparent reason.

This enhancement should be examined in my opinion.
Isn't it close to the reset beanshell interpreter option ?

>
> Then the UI/documentation needs to be updated.
>
> > Vladimir
>


-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message