TechEmpower / FrameworkBenchmarks

Source for the TechEmpower Framework Benchmarks project
https://www.techempower.com/benchmarks/
Other
7.66k stars 1.95k forks source link

Some fraweworks don't follow the rules #6788

Open joanhey opened 3 years ago

joanhey commented 3 years ago

OS (Please include kernel version)

Expected Behavior

Actual Behavior

Steps to reproduce behavior

Other details and logs

joanhey commented 3 years ago

In the cached queries some fw are caching the response. I think that will be good that the test try 2 times the 20 request, and the test need to call 2 times the request for 20 queries If the body is the same is NOT correct

later we continue !!!

Tests: 2 calls to the 20 mulltiqueries and cached if the body is the same, It is NOT CORECT

;)

joanhey commented 3 years ago

https://github.com/vasily-kartashov you are the first but I suspect that others fws do the same

joanhey commented 3 years ago

I'll help https://github.com/vasily-kartashov https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/PHP/hamlet/Benchmark/Resources/CachedQueriesResource.php#L18-L28

joanhey commented 3 years ago

In this test, each request is processed by fetching multiple cached objects from an in-memory database (the cache having been populated from a database table either as needed or prior to testing) and serializing these objects as a JSON response. The test is run multiple times: testing 1, 5, 10, 15, and 20 cached object fetches per request. All tests are run at 512 concurrency. Conceptually, this is similar to the multiple-queries test except that it uses a caching layer.

joanhey commented 3 years ago

NOT a cached response

NateBrady23 commented 3 years ago

Thanks @joanhey

I’m returning to the office in a couple of weeks and I’ll take a look at this.

fafhrd91 commented 2 years ago

Seems "just" framework uses const message object for json bench. I am not sure, I don't have much of js knowledge.

https://github.com/TechEmpower/FrameworkBenchmarks/blob/542904f589b63322b8610b82583ce8fdde97f6d0/frameworks/JavaScript/just/tfb.config.js#L49

billywhizz commented 2 years ago

i think you are right @fafhrd91. i cannot remember why i did it this way but that certainly wasn't intentional. i will create a PR just to use JSON.stringify on a new object.

fafhrd91 commented 1 year ago

@billywhizz just still uses const message

@nbrady-techempower In last test run, redkale performs two times more in updates bench. Looks like a bug in framework or cheating.

volyrique commented 1 year ago

I would not presume malicious intent, given that the author has been responsive to issues like that in the past. Anyway, apparently there is already a fix.

joanhey commented 1 year ago

Check the multiple queries for Redkale https://www.techempower.com/benchmarks/#section=test&runid=8b5c71ba-c738-444e-80d1-581f7e650b68&test=query

It's the first and twice faster than the next fw, and in single query have about 40 faster in front.

NinoFloris commented 1 year ago

Redkale seems to be using an entity cache.

The single and multiple queries benchmark rules don't permit this "Use of an in-memory cache of World objects or rows by the application is not permitted." (see rule xii of https://github.com/TechEmpower/FrameworkBenchmarks/wiki/Project-Information-Framework-Tests-Overview#single-database-query)

Benchmark code: https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/Java/redkale/src/main/java/org/redkalex/benchmark/BenchmarkService.java#L44-L54

For the single query scenario source.findAsync is defined here: https://github.com/redkale/redkale/blob/961ff7fa225cb05273278e01164d7bcb4e03f947/src/main/java/org/redkale/source/DataSqlSource.java#L2606

And the relevant callee of source.findListAsync for multiple queries is defined here (just trace it from findListAsync, it's all the same file): https://github.com/redkale/redkale/blob/961ff7fa225cb05273278e01164d7bcb4e03f947/src/main/java/org/redkale/source/DataSqlSource.java#L3216

This is likely also the source of the updates score discrepancy (given that it also calls into source.findListAsync)

@redkale could you verify whether this is the case?

volyrique commented 1 year ago

As already discussed in #7899 and #7900, it appears that requirement 7b of the cached queries test is frequently violated:

vii. The implementation may use any caching library or approach desired, with the following caveats. [...] b. The implementation should use a bona-fide cache library or server with features such as read through, a replacement algorithm, and so on. Implementations should not create a plain key-value map of objects. If the platform or framework does not ship with a canonical caching capability, use a caching library or separate-process caching server that is realistic for a production environment. A "reference implementation" would use Google's Guava CacheBuilder for in-VM or Redis for separate-process caching.

In particular, the following implementations seem not to comply:

Note that this list is definitely not exhaustive.

Pinging @abahmed, @chrislearn, @Fenny, @fundon, @redkale, and @savsgio for their input.

redkale commented 1 year ago

@volyrique Cache had changed

redkale commented 1 year ago

@NinoFloris Redkale does not use entity caching default, and EntityCache requires the entity class tag @ Cacheable to take effect see https://github.com/redkale/redkale/blob/master/src/main/java/org/redkale/source/EntityInfo.java#L689 such as CachedWorld @Cacheable(direct = true)