You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by jloisel <gi...@git.apache.org> on 2016/08/03 10:43:19 UTC
[GitHub] jmeter pull request #221: Bug 59934
GitHub user jloisel opened a pull request:
https://github.com/apache/jmeter/pull/221
Bug 59934
Fix CssParser race condition and poor concurrency level by using Guava's LoadingCache.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jloisel/jmeter trunk
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jmeter/pull/221.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #221
----
commit 7f21771ab55f4765e2a473f9d457d97d8f00c9af
Author: jloisel <lo...@gmail.com>
Date: 2016-08-03T10:36:56Z
css parser: dramatically improve concurrency level
commit d106461ce60266477f171a242634ce85387d327f
Author: jloisel <lo...@gmail.com>
Date: 2016-08-03T10:37:51Z
move classes to upper package for compatibility
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
https://issues.apache.org/jira/browse/CALCITE-736
>Guava is involved in major dependency hells in Java, with Hadoop the most affected ecosystem.
>Removing Guava types from visible parts of Calcite API will help containing damage to backwards compatibility if/when, at some point, shading needs to be done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
> commons-math3 takes 2.2 MiB and is being used by exactly 4 classes in JMeter. Should we write the statistics computation from scratch to save those 2MiBs? Hell no
That is a nice finding.
We might want to replace commons-math3 with https://github.com/HdrHistogram/HdrHistogram
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
>The idea is JMeter releases are very rare,
This is supposed to change :-)
>CHM has no way to evict old entries, ...
I hope you don't think I ignore that :-) What I mean is create an LRU based on CHM
Another option would be to move to Java 8 and use caffeine directly (1mb).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
>What I mean is create an LRU based on CHM
That is basically what CLHM does. So I would pull CLHM and save our resources for something more valuable than "implementing yet another CHM-based cache for CSS".
> Another option would be to move to Java 8
I'm not sure it is in scope of the current bug id.
Migrating to Java 8 would be indeed nice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
>regarding guava compatibility:
Regarding "guava usage in Apache Calcite project": https://issues.apache.org/jira/browse/CALCITE-1325
>We were incorrect to state that Calcite release 1.8 works with "Guava versions 12.0.1 to 19.0"; it should state "Guava versions 14.0 to 19.0".
>Calcite release 1.7 and earlier do not have this problem.
And so on. The idea is JMeter releases are very rare, and I do not like the idea of making guava a yet another reason to make "urgent compatibility releases".
>Regarding your proposal, there is only 1 contributor to this library, so I am not a fan.
The idea is there is no need to improve the library. All the bugs are resolved long ago, so CLHM serves its purpose well.
CLHM is superseded by https://github.com/ben-manes/caffeine (java8 only) cache.
>I prefer that we just use ConcurrentHashmap then.
CHM has no way to evict old entries, thus if using just CHM, there would be a risk of running out of memory (e.g. when CSS resource URLS are dynamically generated). I do not like exposing JMeter users to OutOfMemory conditions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
>Ok thanks for feedback, I'll commit the PR after further review
+2MiB of dependencies :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
Hello @jloisel ,
Thanks for your contribution.
I agree with your analysis of "race condition" and it impact on performances.
But do you have some benchmark that shows the improvement in performances related to the usage of Guava here ?
Can't the patch be written without it ?
I just want to be sure we absolutely need to introduce guava dependency.
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
> jloisel: Groovy alone takes 7.0MiB
Do you have better language for JSR232 in mind?
The thing is beanshell is outdated, and Nashorn JS is not yet very popular.
In other words: building Groovy in would be visible to end-users. They would benefit from simplified usage of Groovy in JMeter.
On the other hand, if we integrate Guava, then end-users would notice nothing (Guava has nothing to do for day-to-day JMeter usage), except "increased download size".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by ben-manes <gi...@git.apache.org>.
Github user ben-manes commented on the issue:
https://github.com/apache/jmeter/pull/221
> There is only 1 contributor to this library, so I am not a fan.
Please note that CLHM was the precursor to Guava's Cache. While now dormant, its robustness can be attested to by many projects such as Cassandra, Grails, and Neo4j. Due to its small size, many opted to shade it such as Camel, Apache Cayenne, and OmniFaces.
But its purely an LRU cache using the Map interface. Features like expiration, self-populating, etc. were delegated to decorators. This was because a concurrent LRU is surprisingly hard due to every read modifying global state, so it took 2 years of experiments before stumbling on the right ideas. Then Guava took advantage to push the feature envelope and provide a complete caching abstraction.
Guava does have a good deprecation strategy (min 2 years notice). However Hadoop has a poor dependency strategy due to running user supplied jars within its classloader. This lack of isolation causes users to blame Guava, though shading (either by Hadoop or user's) would be an easy resolution.
If you're debating between CLHM, Guava, or Caffeine then you'll probably end up having any bugs fixed by the same developer. Though hopefully by now they've all had enough usage to smooth out the issues.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by jloisel <gi...@git.apache.org>.
Github user jloisel commented on the issue:
https://github.com/apache/jmeter/pull/221
> That basically means if newer Webdriver would use Guava 20, that might render "newer webdriver incompatible with older JMeter".
Well, in 2.13 webdriver plugin was already breaking JMeter by using HttpClient 4.5.2 while JMeter was using 4.2.6. The solution to dependency problem is not to not use any dependency at all. What if you write everything by yourself? You end up maintaining a lot more code, and probably less well written than someone who wrote and maintained a library doing this for years.
> JMeter needs high-performant algorithms
First make it right, then make it fast. Stop optimizing all the time. Optimization comes with a major trade-off, code complexity. Hashing is surely not the bottleneck in JMeter overall performances.
> On the other hand, if we integrate Guava, then end-users would notice nothing (Guava has nothing to do for day-to-day JMeter usage), except "increased download size".
Sure, downloading 2MiB of additional dependencies adds exactly 1 sec with a standard 20Mbits DSL connection. commons-math3 takes 2.2 MiB and is being used by exactly 4 classes in JMeter. Should we write the statistics computation from scratch to save those 2MiBs? Hell no.
> In other words: building Groovy in would be visible to end-users. On the other hand, if we integrate Guava, then end-users would notice nothing.
This is the kind of reasoning which made me leave a software company. Refactoring was considered not adding any value for the end user. Needless to say they are unable to maintain the product anymore today.
When using a library written by somewhere else which does the job for you instead writing the code yourself, it's like "hiring someone to do the job for you". And He takes care of maintaining that stuff, instead of you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
> Hashing is surely not the bottleneck in JMeter overall performances.
Why do you think synchronized CSS cache is the bottleneck then?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request #221: Bug 59934
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jmeter/pull/221
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
@jloisel , my note was not on using ConcurrentHashmap :-) (I am aware of this),
but on the usage of Guava.
Ok thanks for feedback, I'll commit the PR after further review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
>jloisel: Calcite is a database framework whose purpose is to be integrated into java applications
JMeter's purpose is to be integrated with various plugins, so exposing extra core dependencies is not that good.
I'm not sure if we could use OSGi-like approach so "JMeter core" and "plugins" could use different versions of the same library.
That might be good in a long run, however it is not implemented AFAIK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
Regarding "fast cache with limited size", there's
https://github.com/ben-manes/concurrentlinkedhashmap
It is 120 KiB jar with very little possibility to break backward compatibility ever.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
>if you want CLHM, why not propose a PR implementing PR-221 with it ? or do you prefer to wait for decision on guava or not guava ?
I prefer to wait an agreement first, since the code is simple for both cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
@vlsi regarding guava compatibility:
http://stackoverflow.com/questions/21867295/is-guava-binary-compatible-with-previous-versions
Regarding your proposal, there is only 1 contributor to this library, so I am not a fan.
If there's a nogo for guava, I prefer that we just use ConcurrentHashmap then.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
Re guava: AFAIK, if guava is included into JMeter, then it might clash with plugins that also want guava.
AFAIK, guava is known to break backward compatibility quite often, so it might trigger "jar hell" kind of issues.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on the issue:
https://github.com/apache/jmeter/pull/221
I like the idea of the smaller library (cache) instead of rushing into guava for this use case only.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] jmeter issue #221: Bug 59934
Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi
And why don't add guava just for this PR (and not use it in other part of
the code)
And when migration to java 8 will be ready, remove it and use
https://github.com/ben-manes/caffeine
Antonio
2016-08-10 23:08 GMT+02:00 Vladimir Sitnikov <si...@gmail.com>:
> Philippe> I'll wait 2 more days for reaction on guava subject.
>
> -1 for Guava from me.
>
> I would rather speedup migration to java 8.
>
> Vladimir
>
Re: [GitHub] jmeter issue #221: Bug 59934
Posted by Philippe Mouawad <ph...@gmail.com>.
Thanks Vladimir, I am aware of that :-)
Could you comment on my thread.
After further thinking, I don't see why we could not upgrade to Java 8 AND
use guava.
Besides, I think it would be nice to be reactive on the project and
sometimes try things and maybe remove them afterwards.
Anyway, currently in trunk we have a feature (CSS Parsing caching) which
has an issue, if we want to release a 3.1, we need to find a solution.
There is a working PR that introduces Guava (which is largely used so I
suppose it's not bad ) , why not take it as is , fix the problem and if you
want write some note that guava introduction is "experimental" and nobody
should rely on it being in JMeter.
On Wed, Aug 10, 2016 at 11:08 PM, Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> Philippe> I'll wait 2 more days for reaction on guava subject.
>
> -1 for Guava from me.
>
> I would rather speedup migration to java 8.
>
> Vladimir
>
--
Cordialement.
Philippe Mouawad.
Re: [GitHub] jmeter issue #221: Bug 59934
Posted by Vladimir Sitnikov <si...@gmail.com>.
Philippe> I'll wait 2 more days for reaction on guava subject.
-1 for Guava from me.
I would rather speedup migration to java 8.
Vladimir
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
Hello,
I'll wait 2 more days for reaction on guava subject.
Unless there is a nogo, I'll commit the patch as is.
Regards
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
@ben-manes , thanks for those clarifications and your work on CLHM and caffeine, very interesting.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
>Refactoring was considered not adding any value for the end user.
The problem with adding extra dependencies to core is not "JMeter will get harder to maintain"
The main problem (from my point of view) is that typical end-users of JMeter have little to do with Java programming. They have little idea what "NoSuchMethodError" is, and so on.
So, I think in a long run it makes sense to reduce possibility of running into "plugin X is not compatible with plugin Y when deployed to JMeter version Z" kind of errors.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by jloisel <gi...@git.apache.org>.
Github user jloisel commented on the issue:
https://github.com/apache/jmeter/pull/221
Guava is highly stable and mature now, incompatible changes are unlikely to happen. +2MiB of dependencies being added? It's nothing today. Groovy alone takes 7.0MiB in JMeter. Guava is also already used by JMeter Plugins like Webdriver, which also uses Guava 19.0. It's up to JMeter plugins developers to keep up-to-date their plugins, and up to JMeter developers to keep internal libraries up-to-date.
Calcite is a database framework whose purpose is to be integrated into java applications. This is not the general case for JMeter. Problems with dependencies depend on the usage of the library itself.
Caffeine is probably a great caching library which deserves to be used, but it's its only usage. Guava is a multi-purpose library. It has so many interesting libraries like high performance immutable collections, String Joiners and Splitters, Comparable/Comparator made simple through Ordering, Multimaps, Ranges, great hashing algorithms like MurMur, Preconditions and more.
Generally speaking, caches should be avoided at all cost as Martin Fowler said:
> There are only two hard things in Computer Science: cache invalidation and naming things.
[http://martinfowler.com/bliki/TwoHardThings.html](http://martinfowler.com/bliki/TwoHardThings.html)
:+1: for a migration to Java 8 too, as it would allow to use built-in Streams, Function, Predicate and more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
Let's not diverge too much :-)
1. I opened a discussion threads on dev mailing list on Java8 migration.
2. @vlsi OSGI approach is a huge & complex piece of work, frankly I doubt we do it knowing all the other more important things we have on roadmap and that do not make any progress.
3.@vlsi , regarding hashing, can you clarify what you mean in a separate dev discussion ?
4. Regarding JMeter-Plugins/Webdriver, I am not sure we must consider this, it already happened with httpclient dependency. It could happen with a lot of jmeter deps.
5. HdrHistogram should be another discussion, related to CO issue for example on which we discussed a lot without making much progress
6. @vlsi , if you want CLHM, why not propose a PR implementing PR-221 with it ? or do you prefer to wait for decision on guava or not guava ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by jloisel <gi...@git.apache.org>.
Github user jloisel commented on the issue:
https://github.com/apache/jmeter/pull/221
I don't have benchmark. But, according to "Java Concurrency in Practice" in section "11.5 Example: Comparing Map performance", ConcurrentHashmap performs a lot better (up t o 40x faster) than Synchronized map when multiple threads are contented on a single resources.
Here is an extract from the book:
> The major scalability impediment for the synchronized Map implementations
is that there is a single lock for the entire map, so only one thread can access the
map at a time. On the other hand, ConcurrentHashMap does no locking for most
successful read operations, and uses lock striping for write operations and those
few read operations that do require locking. As a result, multiple threads can
access the Map concurrently without blocking.
Synchronized map uses a single lock both for reads and write, contending even read-only threads between each other. On the other side, ConcurrentHashMap uses lock stripping and is contention free between reader threads.
Guava's LoadingCache makes the code easy to understand, easy to maintain and is fast. You can probably be able to write your own cache without guava, but it's hard to implement something both easy to use and efficient in a multithreaded environment.
Guava is the 3rd most popular Java library behind JUnit and SLF4J, i'm actually quite surprised it's not part of JMeter until now:
[The Top 100 Java Libraries in 2016](http://blog.takipi.com/the-top-100-java-libraries-in-2016-after-analyzing-47251-dependencies/)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
> OSGI approach is a huge & complex piece of work, frankly I doubt we do it knowing all the other more important things we have on roadmap and that do not make any progress.
Alternative solution would be to repackage guava (or whatever), so JMeter uses its own org.apache.jmeter.guava...
I do understand that OSGi-like approach is a no way a quick-win.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/221
> like Webdriver, which also uses Guava 19.0
That basically means if newer Webdriver would use Guava 20, that might render "newer webdriver incompatible with older JMeter". That is exactly one of the reasons I'm hesitant to pulling Guava in.
Current case when "Guava is not used by JMeter itself" makes it more flexible for the plugins to use whatever version they like (unless the plugins do not clash with each another)
> jloisel: String Joiners and Splitters
Lots of those has native Java8 classes. I do not say Java 8 makes Guava completely obsolete, however we'd consider each added dependency.
> great hashing algorithms like MurMur
Note: JMeter needs high-performant algorithms, so instead of Guava's MurMur I would use https://github.com/OpenHFT/Zero-Allocation-Hashing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
@vlsi, that was my main concern but @jloisel convinced me, if you have a better option without guava , feel free to amend PR.
Shall I wait or do I commit ?
For now I may not commit the change to URLCollection, only the rest.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by julianhyde <gi...@git.apache.org>.
Github user julianhyde commented on the issue:
https://github.com/apache/jmeter/pull/221
For the record, regarding https://issues.apache.org/jira/browse/CALCITE-1325. That was not an example of "JAR hell". Calcite attempts to maintain compatibility with a wide range of Guava versions and that was a case where our QA process screwed up and we slipped a little. To date no one has ever said "I can't use Calcite because you require Guava X and we use Guava Y".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #221: Bug 59934
Posted by jloisel <gi...@git.apache.org>.
Github user jloisel commented on the issue:
https://github.com/apache/jmeter/pull/221
> Why do you think synchronized CSS cache is the bottleneck then?
I saw a race condition in the existing implementation and fixed it following the [Boy scouts principle](http://programmer.97things.oreilly.com/wiki/index.php/The_Boy_Scout_Rule). It's simply not working properly: the CPU can go crazy if the cache is full when many virtual users are executing. If the requirement is that the code doesn't need to work, then sure, I can make an implementation that gives instant results.
Anyway, I'll be okay with any decision being made, I'm not getting any reward for pushing Guava inside JMeter :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---