Mesh has a compile-time fixed max arena size of 64 GB

Hello
I'm trying to benchmark SPAdes (https://github.com/ablab/spades) with Mesh. Currently SPAdes runs fine with both tcmalloc and jemalloc (and have embedded jemalloc for the sake of completeness). On reasonable small dataset (with memory consumption around ~30Gb) I'm seeing:
src/meshable_arena.cc:448:void mesh::MeshableArena::beginMesh(void*, void*, size_t): ASSERTION 'r == 0' FAILED:

Some quick debugging revealed that mesh tried to mprotect lots of pages of 4k each. As a result mprotect() at some point returns ENOMEM. On my system I'm having:
$ sysctl vm.max_map_count
vm.max_map_count = 65530

If I'd increase vm.max_map_count to 655300 (I'm lucky, I'm having sudo access and the majority of users don't) then the assertion goes away and just std::bad_alloc is thrown. Here is MALLOCSTATS=1 report just in case:
Meshed pages HWM:   297048
Meshed MB HWM:      1160.3
MH Alloc Count:     376537                                                                                MH Free  Count:     629688
MH High Water Mark: 676778

But for me it looks there is some huge design flaw somewhere as the # of memory mappings is a limited resource and one simply cannot mmap / mprotect each page.

Hi @asl!
I think there are ~ 3 things going on here.  First, is that there appears to be a good amount of memory Mesh can reclaim on SPades (over a GB), neat!
Second is that you're right; we're hitting limits around vm.max_map_count.  This is a little confusing and a bug -- we try to explicitly avoid hitting this limit, but I think our existing code to do so is too naiive.  On startup, Runtime::initMaxMapCount() looks at /proc/sys/vm/max_map_count and sets a limit on the number of meshes based on max_map_count.  The comment for kMeshesPerMap says:
// if we have, e.g. a kernel-imposed max_map_count of 2^16 (65k) we
// can only safely have about 30k meshes before we are at risk of
// hitting the max_map_count limit.
static constexpr double kMeshesPerMap = .457;

BUT, we only check that at the start of GlobalHeap::meshAllSizeClasses() -- if we find too many spans to mesh we could end up in the Danger Zone.  I've opened up #38 to specifically track this.
Third, we allocate our (sparse) arena on program startup, which gives us a lot of simplicity.  When things were much earlier in development, the Ubuntu system I was on had trouble with coredumping the arena - it seemed to insist on filling the (mostly empty) virtual mapping of the arena with zeros on a crash.
Before Friday, the arena size was 8 GB, which is too small for your ~ 30 GB working set.  I've increased the arena to 64 GB in the latest commit to master - let me know if this enables SPades to run correctly.  I've opened #39 to track this specific issue.

Mesh can reclaim on SPades (over a GB), neat!

How I can see it? Is this "Meshed MB HWM" value?

/proc/sys/vm/max_map_count and sets a limit on the number of meshes based on max_map_count

Oh, well.. This does not smell good :) SPAdes uses (file) memory maps here and there. Though typically it's something around 10 * # thread, so should be below 1000 for almost any sane system.

Before Friday, the arena size was 8 GB, which is too small for your ~ 30 GB working set. I've
increased the arena to 64 GB in the latest commit to master - let me know if this enables SPades
to run correctly. I've opened #39 to track this specific issue.

I believe it's quite important not have some hard-coded arena size. We could easily utilize, say, 1Tb of RAM in hard cases ;) The actual working set should be around ~60 Gb for this particular dataset iirc. Sadly, now mesh just fails to allocate anything and and therefore throws std::bad_alloc.

More information about that std::bad_alloc – it seems mesh failed to fulfill request to allocate 28 Gb as one piece.
And indeed, in
void *GlobalHeap::malloc(size_t sz) {

We're seeing that it's unable to allocate more than 2Gb of memory in single chunk:
if (unlikely(pageCount * kPageSize > INT_MAX)) {
    return nullptr;
 }

Really? :)

hah, yeah... Thanks for the separate tracking issue :)

And agreed on not requiring a fixed max; it is just that having a single range of virtual address space greatly simplifies the implementation.  I know Go does (or used to do) a similar thing.  This LWN article seems to describe this exact problem: https://lwn.net/Articles/428100/

Hey there. Are there any future plans for fixing this issue? This project has great potential and I can see that a lot of effort has been put into it to solve this "fragmentation" nightmare we all battle against in long-running server jobs. Unfortunately this issue seems to me like a show breaker why one can't use this library.
Could you also explain why that arena size can't be set to "INFINITY" (2^64) ? I mean why it is even needed to have constraint on size? I am not sure I get it why does Ubuntu try to dump memory which wasn't ever malloc'd or was freed.

bump @bpowers

@brano543 can you describe what actual issues you are running into?  The max heap size is now 64 GB, "which ought to be enough for anybody".  Please let us know if you are running into issues with this in practice and we can prioritize working on it, but please don't not try mesh because of perceived limitations.
There are two main reasons we didn't set it larger from the get-go -- the first is that some tools (like the crash reporting software on Ubuntu) choked on very large virtual memory mappings.  The behavior we were seeing was: Mesh would allocate 64 GB of virtual address space.  A program would allocate a few hundred MB and then crash.  The core dump parser wouldn't be smart enough to understand 63.5 GB of that virtual address space wasn't ever allocated or backed by real RAM, and would try to create a core dump file for sending to Ubuntu filled with 63.5 GB worth of 0s.  There is a madvise flag MADV_DONTDUMP that should help with this, but at the time, I had trouble integrating this in a way that didn't hurt performance.
The second reason is that we have some ancillary data structures we allocate (like lookup tables) that depend on the size of the arena.  I think this is a smaller issue, as they will "just" use up some extra virtual address space.

Thanks for clarification. This effectively closes Mesh for SPAdes as we're routinely allocating more than 64 Gb of RAM. Apparently no other memory allocator we are aware of has such a limitation.

How much memory do you allocate? I feel like this is something that could be made a build-time parameter.

As much as necessary. Could allocate 0.5 Tb, could allocate 1 Tb.

And to be clear, you mean that the actual physical footprint of the app in RAM is ~ 1TB, correct?

It might be 100 Mb, it might be 4 Gb, it might consume 1 Tb. Everything depends on the input.

@asl if you increase this constant here: https://github.com/plasma-umass/Mesh/blob/master/src/common.h#L104 from 64 to 2000, that should bump the max heap up to 2 TB.  I would be eager to hear how this works for you!  If things seem to work fine, I can do some testing on some much smaller systems, and see about making that the default.

I'll also talk to @emeryberger - my intuition is that having a single, non-growable heap makes parts of the implementation significantly easier, but maybe I'm overthinking it.

Well, for us we'd need something like a run-time constant then. E.g. the user could specify max amount of memory he / she could use.

@bpowers So, I tried again on small dataset (with expected memory consumption less than 10 Gb). Unfortunately, I had to lower kMeshesPerMap down to 0.1, otherwise it did not work. So, I guess #38 is really a blocker.

I also just ran into this as well:
Mesh: arena exhausted: current arena size is 64 GB; recompile with larger arena size.

The system has 128 GB RAM and the application uses more-or-less 128GB RAM for the given input.
I would also like to test it on another machine with up to 1TB if RAM (and use all of it).
I have three questions:

Like the error message suggested, I changed the arena size here https://github.com/plasma-umass/Mesh/blob/master/src/common.h#L124 to 128, i.e. static constexpr size_t kArenaSize = 128ULL * 1024ULL * 1024ULL * 1024ULL; . Is this correct? Do I need to change anything else?
Does the issue with the (ubuntu creating mostly empty memory dumps)[https://github.com/plasma-umass/Mesh/issues/37#issuecomment-533962806] still exist?
Why not check the amount of installed system memory at compile time and set parameters so that all of it can be used (or slightly higher)?

For a few projects (actually implementations of programming languages for HPC etc.) I wanted to propose to use Mesh. But any such compile-time limitation puts me off. It's simply impossible to use - many ordinary desktop systems have more than 64GByte ram nowadays. Ordinary servers typically several terabytes and special systems tens or even small hundreds of terabytes.
Any plans on removing this limitation or making it at least a run-time setting?

@dumblob The project seems to be abandoned. We (SPAdes) are using mimalloc now.

@asl aha, ok. That's a pity anyway.
Btw. actually I wanted to test Mesh against mimalloc and I'm pleased to hear mimalloc serves your purpose well (my experience is also basically only very positive with mimalloc).

The lead PhD student on this project, @bpowers , has moved to industry and recently had a child, so he has been otherwise quite occupied :).
In any event, this particular issue got lost; sorry about that.

I see - then I wish all the best to @bpowers et al.
Should this project get resurrected at some point, I'll try to keep an eye on it.


Datarace

When launched from two different test functions it produces a datarace in the file path memory site, at least.
https://github.com/botika/yarte/commit/48ccdea6caa7f00be0680eb5b77e47a7bebaa037

I believe this was fixed in #114.


Composition retrieved misses several items for each slot archetype with respect to the one posted

Configuration information

404
{"timestamp":"2021-02-01T16:16:54.691+00:00","status":404,"error":"Not Found","message":"No message available","path":"/ehrbase/rest/openehr/v1/status"}

EHRbase version:0.14.0 dockerized
openEHR_SDK version:
Archie version:
PostgreSQL version:
Java Runtime version:
Operating System version:Ubuntu 18.04

Steps to reproduce
I have a similar problem to https://github.com/ehrbase/ehrbase/issues/386 but it happens both posting the composition in xml and json. When the composition is retrieved it misses all the items inside the slot archetypes but one. (It happens both in the representation shown when I post the composition and in the persisted one retrieved via REST GET)
At the following link:
link to files
I pasted:
-the opt for the compositions. its name is Case4.opt
-a json composition named case.json
-the retrieved json composition named case_retrieved.json
-a xml composition named case2.xml
-the retrieved json composition named case2_retrieved.json
For example comparing the json composition with the one retrieved in three points I have an archetype which has more than one item but only one is retrieved, e.g. Archetype openEHR-EHR-CLUSTER.multimedia.v0 has at0001, at0002, at0008, at0006, at0009. Only at0009 is returned

POST template
POST (one of the two) composition
retrieve (GET) composition
compare the retrieved with the posted
Actual result

Composition posted and retrieved differ
Expected result (Acceptance Criteria)

The composition should differ only for items order
Definition of Done

[ ] The defect is checked by an unit or an integration test (Robot)
[ ] Merge Request approved
[ ] Unit tests passed
[ ] Build without errors
[ ] Release notes prepared
[ ] No additional runtime warnings

I add that I checked the DB and the compositions are complete there. There aren't missing items.

I add that I checked the DB and the compositions are complete there. There aren't missing items.

I tried the latest build and the problem report is solved (both in xml and json)


[NLPCRAFT-327] Fixed .nlpcraft directory not present issue

JIRA: NLPCRAFT-327
When I first built NLPCraft, the .nlpcraft folder did not auto-create. This code fix will make NLPCraft run without an error by auto-creating the .nlpcraft folder if not already present.

Fixed in code  - look at https://issues.apache.org/jira/browse/NLPCRAFT-327


Notion template link does not work

System Health details
2023.02.01
Checklist

[X] I have enabled debug logging for my installation.
[X] I have filled out the issue template to the best of my ability.
[X] This issue only contains 1 issue (if you have multiple issues, open one issue for each issue).
[X] This issue is not a duplicate issue of any previous issues..

Describe the issue
you require users install a template to try this integration but the link to said template doesnt work.  There also is no clear "winner" for the Todo list you intended when I search the marketplace on their site
Reproduction steps
click the url and watch it fail
Debug logs
n/a

Diagnostics dump
n/a

seems there is a PR ready with a fix so I wil close this as done
https://github.com/JanGiese/notion_todo/pull/20
Thanks


rasa dynamodb putitem error because of dynamodb item size increased more than 400kb

Rasa Open Source version
2.8.9
Rasa SDK version
2.8.2
Rasa X version
0.42.3
Python version
3.7
What operating system are you using?
Windows, Cloud (AWS / GCP / AZURE)
What happened?
i am using rasa dynamodb tracker and when my rasa chat session is closed and the user try to communicate again the same sender id is assigned these leads to an issue as the chat data keeps on growing causing the item size to increase in dynamodb as dynamodb has a maximum capacity of 400 kb for a single item. these causes the action server to fail and causes issues in my slack bot application.
Command / Request
kindly give a appropriate resolution to the issue

Relevant log output
No response

Thanks for raising this issue, @rctatman will get back to you about it soon✨
Please also check out the docs and the forum in case your issue was raised there too 🤗

I believe this is actually working as intended: we do want the same sender continuing to get the same sender id across conversations. If you'd like help with your dynamodb implementation I'd point you towards the forums instead: https://forum.rasa.com/


Correct unit of watch interval used to detect keycloak config map changes

I tested it myself. Confirmed that it is possible to turn the new watcher off by using a zero interval.


Feat: Adding surveys to make it more interactive

Describe the feature
I would like to add a feature where new users could do some surveys it will make website more interactive.
Add ScreenShots
Like this:

Record

[X] I agree to follow this project's Code of Conduct
[X] I'm a GSSOC'24 contributor
[X] I want to work on this issue

Please assign me this issue under GSSoC'24

I would  Like to work on this. Please assign this issue to me

@Chelseasingla1 Please assign to me task and solve the problem.

This feature is not required


NewDeviceMessage redesign

@cehali Rebased

Broken when displaying on wallet's funds page:

@Marik-D Could you check this PR again?


Investigate Web Assembly (WASM)

It seems that a reasonable proportion of the time spent in executing pdftex is spent in parsing and initialisation, and as such WASM may reduce execution times noticeably, especially for small files.

Seems to not be in Node yet: https://github.com/nodejs/NG/issues/19#issuecomment-269054557

As referenced in the above thread, builds can be downloaded for ubuntu here:
https://build.chromium.org/p/client.v8.fyi/builders/V8 - node.js integration
That work, no --expose-wasm flag required.


Please consider get rid of the login when app is opened

As I always close that activity but it always shown to me.
This is really inconvenient and not good for user experience.
Please consider put it in a optional function.
Thanks!

Thank you, I agree. What version of the app are you running?
In recent versions, there's an anonymous login, and in the settings screen, you can check "always anonymous" and the login screen will be skipped every time. In the other hand, if you sign in once, the app will remember your credentials and will likely skip the login screen too.

The version is 2.3.6
I saw that setting after you told me.
Thanks!


Search: Results Page

Once the designs are final(-ish), a search results page should be built.
See https://app.zeplin.io/project/59fa2311bac7acbc8d953da9?seid=5b51788f2ad605b11e38f756

This includes:

search bar

search box
listing type picker on left side of search box
price filter
availability filter for fractional usage listings
links with pre-defined filters/sorting on the right of the search box
sort by button (price low to high, most to least recent creation date, ...)

displaying search results based on actions from search bar

Note: the "location picker" component is not included in this task since it is a bit more complex. It will be tracked as separate issue #366

Resolved by https://github.com/OriginProtocol/origin-dapp/pull/396 ✅


Add a integration test for large asset counts

Issue Number

Relates to #2552
Overview

[x] Add integration test for covering the border in-between where we don't need to split token-bundle change, and where we do need to, to ensure we have the calculation correct

Comments
stack ghci cardano-wallet-core-integration cardano-wallet:integration

λ> import System.Environment
λ> setEnv "NO_POOLS" "1"
λ> :main --match SeaHorse

bors try

bors r+

Need to update the two Transaction with too much metadata tests after increasing the max tx size...

bors r+

bors r+

bors r+

bors r+

bors r+

🤔
bors r+


Resizing iframe will cause lose drag-end event

Describe the bug
A clear and concise description of what the bug is.
When I use a iframe in as-split-area, drag gutter to the iframe side fast and release the mouse button, gutter bar will move with mouse all the time.
"angular-split": "^3.0.1".
To Reproduce
Create a Stackblitz demoing the bug/issue:
open https://stackblitz.com/edit/angular-rcqyv1
and replace app.component.html content with

	<as-split direction="horizontal">
		<as-split-area size="40">

		</as-split-area>
		<as-split-area size="60">
		</as-split-area>
	</as-split>

Dragging the gutter to left side fast and it will lose the onmouseup or drag-end event. After that angular-split will keeping the dragging status.

Hi @Mike-Jia
Check this demo to solve that:
https://bertrandg.github.io/angular-split/#/examples/iframes
https://github.com/bertrandg/angular-split/blob/master/src_app/app/component/examples/iframe.route.component.ts

Hi @bertrandg
Thanks for your solution, it's work now.


Fhmobsdk 53 local fh params

This change does two things. First it always attempts to load fhconfig.json regardless if it is local (defined as having a query parameter "url" set). Second it allows any of the fh-params to be overridden from a query parameter. I added the second part because I think it answers the first attempt at allowing a host override more generically. We might consider prefixing them with "fh-". It seems somewhat obtuse to just have a parameter of "url" which may collide with someone's app code.
This change fixes issue https://issues.jboss.org/browse/FHMOBSDK-53

@shannonmpoole this looks good to me. I think @wei-lee has made some improvements to how we release SDK updates, so I'll let him weigh in on that.
Just on the url param change - I'd be in favour of it (we should've gone with it at the start), but there's quite a few changes involved in it:

Update all Gruntfiles in our client templates (they use grunt connect to serve and open your client with a ?url= param right now, would need to be changed
NGUI updates to use fh-url (or whatever param we decide on) for the preview
The actual SDK change (which would also need to maintain backwards compatibility for ?url=)

From now on, to deploy the SDKs to the platform, you will need to go through these steps:

Upload the built/dist files to the Github Releases of this repo. See the current release for example (https://github.com/feedhenry/fh-js-sdk/releases). Make sure to use the same naming conventions as the current release. This is still a manual process for now, but eventually the releases will be created automatically when a new tag is created.
Create a PR in fh-mobile-sdks with the updated version number in makefile.
Create a change request to ask ops to deploy fh-mobile-sdks component - you will need to create a PR in fhcap.
Once deployed, close all the PRs.

Since I am still in the process of deploying the fh-mobile-sdks for the first time, once this PR is closed, I will handle the deployment this time.

@jasonmadigan Yea I figured as much about the backwards compatibility and the changes needed for the preview and gruntfiles. Right now this pull request does maintain backwards compatibility for url, maybe we should just prefix the other params now? Then if/when we change the sdk and gruntfiles it will be done correctly. So to be clear, leave the url param as is (override host) but apply any other prefixed (fh_) url params. So url or fh_host would override host. Honestly, right now it's just additional functionality and shouldn't affect current implementations, I just thought it made sense while I was making this fix,  which was supposed to be simple and I don't want to spend too much time worrying about this extra stuff so feel free to just say scrap it.
@wei thanks just let me know anything I can do to help or make future pull requests go more smoothly

@shannonmpoole I'd say if it's not too much extra effort for you to add support for both now, go for it!

@jasonmadigan Not at all, done :) I think it makes much more sense now anyways.
http://localhost:9002/?url=http://localhost:8001
is equivalent to
http://localhost:9002/?fh_host=http://localhost:8001

And you could now do
http://localhost:9002/?fh_host=http://localhost:8001&fh_connectiontag=1.2.3

@shannonmpoole could you please rebase your branch using FHMOBSDK-57-support-mbaas-auth branch and create a PR against it? I will handle the release of that branch.

Rebased in #119


Npm publish rights please?

Hey @wyze, I've just updated the repo to use the latest reason-react. Can you gimme the npm publish rights so that I can release a new version please? Thanks.

Done. Thank you!


[ENHANCEMENT] [GUI] Port the cloud credentials expiration feature

Is your enhancement related to a problem? Please describe.

Original issue: https://github.com/rancher/dashboard/issues/11256
Describe the solution you'd like

We have to make a porting from Rancher to the Harvester extension.
Describe alternatives you've considered

Additional context

Closing as the Cloud Credentials feature does not involve the Virtualization Management page


[1.x] Supervisor: multiple processes without Sail publish.

Problem
If you need run cron or horizon or any process all time, you must publish sail runtime directory and change supervisord.conf
Solution
Append docker-compose.override.yml with config:
version: '3'
services:
    laravel.test:
        volumes:
            - './docker/php/cron.conf:/etc/supervisor/conf.d/51-cron.conf'
            - './docker/php/horizon.conf:/etc/supervisor/conf.d/52-horizon.conf'

And run supervisord with native config but not Sail's custom. Because native config run all configs from conf.d
Changes
Deleting default supervisord configs because this exists in /etc/supervisor/supervisord.conf and nodaemon appends in start-container script
- [supervisord]
- nodaemon=true
- user=root
- logfile=/var/log/supervisor/supervisord.log
- pidfile=/var/run/supervisord.pid

Just use publish.

Just use publish.

Why? It’s more clearly then publish


Add CLI

I'm also considering either scraping TheIphoneWiki or including a JSON file to store all encryption keys.
This is in continuation to #4

I'm gonna make the CLI more feature complete before merging, so it may be a while.

You mean to the inclusion for both serializing and deserializing of all permutations? I can add this also.

Something like:
pyimg4 im4p create FILENAME DATA [--compression ..] [--key. ..] [--iv ...]

You mean to the inclusion for both serializing and deserializing of all permutations? I can add this also.

Yeah. I'll probably work on adding some commands tonight.

I think the CLI should be ready to merge now. @doronz88 do you think there's any other features that should be included?

Just for the git history sake, I think its better to rebase it now before merging


Speed up chunkcopy and memset

This was found to have a significant impact on a highly compressible PNG
for both the encode and decode.  Some deltas show performance improving
as much as 60%+.
For the scenarios where the "dist" is not an even modulus of our chunk
size, we simply repeat the bytes as many times as possible into our
vector registers.  We then copy the entire vector and then advance the
quotient of our chunksize divided by our dist value.
If dist happens to be 1, there's no reason to not just call memset from
libc (this is likely to be just as fast if not faster).

Nice work!

So turning on asan revealed some overlapping memcpy's occurring.  Modifying the memcpy routine to instead by memmove now has me at a dead end, with some things referring to zero pages.  This is...odd.

@mtl1979 so using the old chunkcopy_safe makes everything work again on the pigz tests, despite them being functionality equivalent.  The address sanitizer is detecting overlap in the regions fed to memcpy, but switching memcpy out for memmove gets rid of that warning but then still errors out with this:
=================================================================
==30780==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000c8 (pc 0x557ddbb9b113 bp 0x000000000021 sp 0x7ffc7881fec0 T0)
==30780==The signal is caused by a WRITE memory access.
==30780==Hint: address points to the zero page.
    #0 0x557ddbb9b113 in try_throw_ (/scratchdir/adam/zlib-ng-bench/test/pigz/pigz+0x33113)
    #1 0x557ddbb98949 in infchk (/scratchdir/adam/zlib-ng-bench/test/pigz/pigz+0x30949)
    #2 0x557ddbb9a05c in process (/scratchdir/adam/zlib-ng-bench/test/pigz/pigz+0x3205c)
    #3 0x557ddbb8aa24 in main (/scratchdir/adam/zlib-ng-bench/test/pigz/pigz+0x22a24)
    #4 0x7f0c3d9a0789  (/lib64/libc.so.6+0x2b789)
    #5 0x7f0c3d9a0838 in __libc_start_main (/lib64/libc.so.6+0x2b838)
    #6 0x557ddbb8b540 in _start (/scratchdir/adam/zlib-ng-bench/test/pigz/pigz+0x23540)

As in, there's probably some pointer arithmetic computed from a null pointer.  So, are we somehow relying on the side effects of the overlapping memcpy ranges?  Are we just getting lucky by doing these chunks at a time?  I guess I could work around this looking for the overlapping range but that doesn't seem right.

@KungFuJesus It's almost like some variable was not initialized correctly anymore and as result some pointer difference was calculated from NULL instead of correct value... The diff is so much a mess, that it's hard to see where the variable is not initialized correctly...

@KungFuJesus It's almost like some variable was not initialized correctly anymore and as result some pointer difference was calculated from NULL instead of correct value... The diff is so much a mess, that it's hard to see where the variable is not initialized correctly...

So I have a rough idea of what's going on.  The uint8_t *from argument is declared const, but clearly there's some overlap (probably on front).  I'm wondering if the compiler is making assumptions about that pointer to supposed consts.
Everything just works if I put the old chunk_copy_safe back in.  Granted, that's not the most reliable canary but it really seems like it is.

@KungFuJesus With a quick look at just chunk_copy_safe, I could see two related issues... It's possible out wraps around, because it's not consistently updated inside or outside the function.

I'm not entirely sure what you mean.  The out and from pointers overlap in the case that things go awry.
int olap_src = ((from >= out && from <= dst_end) ||
              (src_end >= out && src_end <= dst_end));

if (!olap_src) { doOldBehavior(); } seems to fix things.  The fact that memmove doesn't fix this makes me think we're relying on the behavior of the chunked copies to overwrite the bytes in this order or we're getting really lucky with it.  What you just said sounds like a circular buffer but that only makes sense in the context of wrapped indexing - as far as this function is concerned we're just looking at linear segments of memory here.

@KungFuJesus The whole point of chunkcopy is that things will likely overlap when reading, that's why we use temporary variable to store the chunk. When we write the chunk to destination, we make sure we don't overrun the buffer. We use largest usable length to avoid speed penalty of small copies using general-purpose registers.

@KungFuJesus The whole point of chunkcopy is that things will likely overlap when reading, that's why we use temporary variable to store the chunk. When we write the chunk to destination, we make sure we don't overrun the buffer. We use largest usable length to avoid speed penalty of small copies using general-purpose registers.

Yes but memmove should basically be doing the same thing in this circumstance as far safety is concerned.

@mtl1979 Latest commit makes it work but only if in the circumstance that we have overlap with the source that we copy in chunks of >= 8 bytes at a time.  Any less than this (which with zmemcpy_* should be doing scalar GPRs copies), and it still fails.  I think there's a bug hiding here but I'm at a loss for where exactly because I don't know the internals of this compression algorithm that well.

@KungFuJesus It might be unnecessary to define and undefine CHUNK_SIZE there... Most compilers already can use multiple smaller registers to copy when larger register type is not available... The code isn't much better as of now than the original chunkcopy_safe through functable.

@KungFuJesus It might be unnecessary to define and undefine CHUNK_SIZE there... Most compilers already can use multiple smaller registers to copy when larger register type is not available... The code isn't much better as of now than the original chunkcopy_safe through functable.

It is though, as the !olap_src condition seems to be hit pretty often. The code size is about the same but the performance of that memcpy is way higher than it was when we were synthesizing our own memcpy with vector sized chunks.
I agree, I can probably get rid of all the ifdefs in there and just let it do everything from 32 bytes down. It'd probably produce better code even, as even on pre-avx hardware it's likely to at least split the copy over two xmm registers in the same clock cycles.
As for why it's outperforming the original chunkcopy_safe, well that mystery may be possibly shrouded in what I suspect is a bug that relies on the side effect of the overlap here in some way. I suspect overlapping regions has some part in it, though.

@KungFuJesus Cleaning up pull requests is usually the most annoying thing, but it makes the code look a lot more professional and also easier to read.

@KungFuJesus Cleaning up pull requests is usually the most annoying thing, but it makes the code look a lot more professional and also easier to read.

I fully intend to, what's there for chunkcopy_safe right is kind of a nasty mess.  I can remove pretty much all of the preprocessor statements in there as a start but what's more troubling to me is why this function is failing under these circumstances.  The segfault that's happening on the clang build with optimizations disabled maybe is another piece in the puzzle here.

I fully intend to, what's there for chunkcopy_safe right is kind of a nasty mess.  I can remove pretty much all of the preprocessor statements in there as a start but what's more troubling to me is why this function is failing under these circumstances.  The segfault that's happening on the clang build with optimizations disabled maybe is another piece in the puzzle here.

Finding any regressions is a lot easier when the code is clean... Then you can start adding printf's to see where things go south...

If I am remembering this correctly, we can't use memmove because the full first chunk might not be in place by the time it starts copying chunk-wise. Like if the dist=4 and chunk size is 8, it first needs to copy 4 bytes first, then it can do chunk-wise copying - something which I don't think memmove does. Maybe that applies here, maybe not.

I mean this works if I do a bytewise copy loop and that does have defined behavior, sort of.  What we're doing is something like this:
[from0, from1, from2, from3, from4]
                        [out0,  out1,   out2,   out3]

Where the positional overlap is there indicated above and copying the value such that by the time we're at out2, we've altered the input.  This really seems like it'd be a bug though, you don't usually intend to have your copy loop alter the contents mid copy.  The code functions correctly when I do, but in a way this would basically make > than byte sized copies, such as what might be used on an optimized memcpy or what we're doing through GPRs, have some unexpected behavior based on the buffer start and stop positions.  In that way, the use of memcpy and anything greater than byte by byte is going to risk being incorrect.  We are likely just getting lucky by doing this if that were the case (or we have perfect overlap positions such that this never happens incorrectly).

I think this is more or less the body of code that the copychunk_safe function is emulating:
            if (copy > left) copy = left;
            left -= copy;
            state->length -= copy;
            do {
                *put++ = *from++;
            } while (--copy);

I think "left" is the *safe in this context and yes the copy loop seems to be byte by byte.  I guess only Mark Adler would know whether or not it's intentional but I could see this maybe being possible.  For now I guess I can emulate this behavior by properly capture everything in the overlap :-/.  This really feels weird, though.

Can't we just do a byte copy loop up to sizeof(chunk_t) and then use memcpy after that? I am thinking that should work.

Can't we just do a byte copy loop up to sizeof(chunk_t) and then use memcpy after that (starting at the original byte copy position)? I am thinking that should work.
Hmm. But it might not work if memcpy is using larger chunk size internally than our sizeof(chunk_t). We could do byte copy loop up to 32 bytes (AVX 512? which is the max chunk size on x86?) and then use memcpy after that.

64 byte copies are what avx512 is technically capable of (though we're only using ymm-sized registers), but that is not going to sufficiently capture the side effects in the overlap as a byte for byte copy would.  Memcpy also could be bypassing registers entirely if it's using ERMS.  The side effect of this self-modifying copy is difficult and hazardous in all > than byte sized copies simply because your chunk size may not be an even modulus of the offset of the overlap.  Then it doesn't actually matter if the copies pass through registers or not, you're partially losing a modifying byte overlap when you atomicity is only 32/16/8/4/2 bytes.
I think the proper way to do it is to first copy the overlap contents to the beginning of the buffer, then modify the contents of where the overlap occurs (the entire sliding window), then I think you can probably copy the rest with memmove.  But I'd have to think carefully on it.  Some confirmation that this overlap is 100% intentional for this copy would also be helpful in case we're missing a bug that happened to be introduced by the caller unintentionally.

I'm kind of wondering if we're hitting this corner case in the comments:
                         if (op < len) {         /* some from end of window */
                              len -= op;
                              out = chunkcopy_safe(out, from, op, safe);
                              from = window;      /* more from start of window */
                              op = wnext;
                              /* This (rare) case can create a situation where
                                 the first chunkcopy below must be checked.
                               */
                          }
                      }
                      if (op < len) {             /* still need some from output */
                          len -= op;
                          out = chunkcopy_safe(out, from, op, safe);
                          out = functable.chunkunroll(out, &dist, &len);
                          out = chunkcopy_safe(out, out - dist, len, safe);
                      } else {
                          out = chunkcopy_safe(out, from, len, safe);
                      }

So I don't really trust this solution, and I do suspect there's a bug hiding in here but I'm not introducing a new one by falling back to the old behavior when overlap is detected.  The extra bounds checks are pretty quick and the memcpy makes things definitely worthwhile (we often call into this function without overlap).
This latest push works and seems to pass muster with the CI.  I do think we should take a closer look at that this code.  There's likely a way to compatibly simulate byte for byte copy with 3 memcpy's and a whole bunch of boundary checks but this happens infrequently enough and fixing it the right way is hurting my brain.

Removing the chunkcopy_safe_stub should reduce code size. +1

Hmm, still not sure what's up with clang noopt. I'll have to take a closer look at that.

Hah so a realization: WITH_OPTIM being off disables all the chunkset stuff which is probably why it's uncovering a potential bug here.  Adding in a straight byte for byte copy here makes things work, even with optimizations disabled.  So this is definitely at least the immediate cause of the bug.  Why exactly I haven't sussed out but I guess for now I can macro around it and do the 8 byte case for WITH_OPTIM=OFF like we were before.

@nmoinvaz I just pushed the idealized and safe form of this function for overlap.  It should mimick perfectly and safely a self modifying copy loop.
The non overlap distance of the two strings virtually behaves like a safe lookahead/lookbehind factor.  After we do the initial non-overlapping bytes each subsequent traversal of the non-overlap width is both by definition non-overlapping and safe to do.  This also doesn't have undefined behavior in memcpy so it won't set off sanitizers.  While we could have made this loop call memcpy directly with non_olap_size, if non_olap_size is small, we have a really non-ideal version of this function that repeatedly actually dispatches to libc.  By using fix widths up to non-olap size, we don't have any function calls will still having a semi-performant loop.

@KungFuJesus I think that originates from the Intel fork actually, so I am not actually sure why that is needed.
This is the commit that introduced that code:
dc3b60841dbfa9cf37be3efb4568f055b4e15580

Interesting - that actually makes a fair bit of sense.  We're sacrificing deflate performance by reorganizing the inflate window so that the checksum can occur at the same time as the crc for the case of gzip.
This also paves the way to folding an adler checksum and inflate copy together, though we may be paying more for it on this niche of extreme compressibility.
I might want to try things with larger files that don't compress as well to find out if this is just a one off unrealistic case.  I do have a couple of other optimizations in mind that can probably ride along with this PR - I'll let you know once they are fleshed out.

Another interesting result of this is that they are compressing better for levels 1-6 than us, at least for this RGB stripe image.  This is of course deflate, not inflate, but it does mean that they are working with fewer bytes to begin with and makes this not a direct comparison.

This also paves the way to folding an adler checksum and inflate copy together, though we may be paying more for it on this niche of extreme compressibility.

One thing I found out was that crc32_byfour_with_copy was actually slower than just crc32_byfour + memcpy. So it really only makes sense to do the dest copy with large vector sizes otherwise memcpy will outperform.
It is possible with our current code that with -DWITH_OPTIM=OFF we could be slower on adler32 paths, because I think there is that extra copy that occurs with memmove and from what I remember the original code just copies straight into strm->next_out

I should note that's with each prospective implementation's deflate run beforehand.  I need to instead compress things with the stock zlib entirely for a fairer comparison so that we can just look at inflate performance.  Though, the fact that chromium is deflating more makes a bit of sense why our deflate is faster.

With identical files they are still about twice as fast in a lot of inflation scenarios:
Running ./benchmark_zlib_apps
Run on (28 X 2989.91 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x14)
  L1 Instruction 32 KiB (x14)
  L2 Unified 1024 KiB (x14)
  L3 Unified 19712 KiB (x1)
Load Average: 0.92, 0.70, 0.44
--------------------------------------------------------------------------------------
Benchmark                                            Time             CPU   Iterations
--------------------------------------------------------------------------------------
png_decode_realistic/png_decode_realistic/0      19206 us        19206 us           36
png_decode_realistic/png_decode_realistic/1     123324 us       123321 us            6
png_decode_realistic/png_decode_realistic/2      83232 us        83230 us            8
png_decode_realistic/png_decode_realistic/3      52754 us        52752 us           13
png_decode_realistic/png_decode_realistic/4      74819 us        74817 us            9
png_decode_realistic/png_decode_realistic/5      59547 us        59544 us           11
png_decode_realistic/png_decode_realistic/6      47644 us        47643 us           15
png_decode_realistic/png_decode_realistic/7      44288 us        44287 us           16
png_decode_realistic/png_decode_realistic/8      40725 us        40723 us           17
png_decode_realistic/png_decode_realistic/9      36371 us        36370 us           19
adam@eggsbenedict ~/scratch/zlib-ng-bench/build/test/benchmarks $ ./benchmark_zlib_apps_alt --benchmark_filter=png_decode_re
2022-02-24T13:12:02-05:00
Running ./benchmark_zlib_apps_alt
Run on (28 X 4101.42 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x14)
  L1 Instruction 32 KiB (x14)
  L2 Unified 1024 KiB (x14)
  L3 Unified 19712 KiB (x1)
Load Average: 0.79, 0.69, 0.45
--------------------------------------------------------------------------------------
Benchmark                                            Time             CPU   Iterations
--------------------------------------------------------------------------------------
png_decode_realistic/png_decode_realistic/0      16631 us        16630 us           42
png_decode_realistic/png_decode_realistic/1      55356 us        55355 us           12
png_decode_realistic/png_decode_realistic/2      48412 us        48410 us           14
png_decode_realistic/png_decode_realistic/3      43984 us        43983 us           16
png_decode_realistic/png_decode_realistic/4      53160 us        53158 us           13
png_decode_realistic/png_decode_realistic/5      54965 us        54964 us           12
png_decode_realistic/png_decode_realistic/6      50274 us        50273 us           14
png_decode_realistic/png_decode_realistic/7      48169 us        48168 us           15
png_decode_realistic/png_decode_realistic/8      47803 us        47802 us           15
png_decode_realistic/png_decode_realistic/9      45444 us        45443 us           15

@KungFuJesus You need to disable CPU throttling to get comparable results... Of course zlib-ng will be slow as hell if the CPU is running at half the clock rate... That's one of the reasons why I don't do benchmarks as some of my BIOS firmwares have no option to disable CPU throttling...

I'm running in performance mode, there's no governor, it's full blast.  The profile definitely tells me that we are the caller of memmove that's so expensive (heh, after finally remembering to disable the omission of frame pointers):

As you can see, that memmove is the majority of time.  That's for running inflate on a level 9 PNG (our synthetic one).

What's funny is when reading in the precompressed images into memory and benchmarking that, these memmove's become far less of a tent pole (somewhat expected, as a lot of time moves to undoing the compression filter in libpng).  When we're in that scenario, it looks like for level 8 we're like 11% worse.
I think level 0 is kind of a weird thing that's somewhat uncommon.  Other than wanting to be faster at decompression I don't see it having a ton of applicability and is perhaps not that useful to worry about in a benchmark.  Though it does manage to measure things without the effects of some of our more expensive functions so in that regard it's kind of interesting.  It's more akin to a memcpy/memcpy surrogate benchmark at that point.
Still, this tradeoff for the running checksum is pretty apparent.  I also don't believe we're seeing any benefit from it on non-gzip workloads, as they are not leveraging the CRC fields in the stream.  It's trivial how we'd integrate the adler checksum into a copy here and is perhaps worth trying, but it's interesting that we're losing on the memmove rather than the memcpy that occurs earlier.  I'm guessing that memmove is probably moving way more bytes than what's happening in that window checksum routine.

Another thing I thought of while looking at all of this - while the chunkset templates can use the base AVX instructions for doing those vectorized moves, it may not be the most beneficial thing for pre-haswell architectures.  Everything pre-haswell actually had to span two loads and stores to emulate a full 256 bit move.  If we're only building with -mavx with those, well, the benefit may not be much greater than building with -msse4.

So we definitely shouldn't hold up this commit on this but, some relevant observations:

For whatever reason, likely unrelated to the Intel CRC folding optimization, we're getting crappier compression ratios at lower levels than the chromium zlib fork (and I think stock as well).  This accounts for some of our better compression speed which is perhaps something that disqualifies timing benchmarks a bit
We're getting poorer inflate numbers even on the same files.  This seems likely related to the Intel folding CRC optimization.  It's worth writing a parallel inflate that perhaps operates similarly to the pre-optimized version to test that theory.  If that does check out, maybe we can find a way that is not so messy to merge the two techniques or find when the folding checksum bits are advantageous (obviously we aren't getting any gains on the adler checksum since it just does a full memcpy first and then checksums the window).  While folding the copy into the checksum will obviously buy us some gains there, I don't suspect it'll pull us out of the predicament that we're in with memmove dominating a lot of the time rearranging the order of the windows when dealing with super highly compressed data.  Of course, I could be completely wrong, so an experiment will need to be divised to test this theory.
Someone else that is super knowledgable with regard to the mechanics of inflate may likely be a better candidate to devise this test.  The benchmarks in #1170 should help with this.  As far as deflate performance goes, if you modify the encode_png test fixture's "teardown" method to flush each of the buffers to a file you should be able to see the comparison for the compression ratio.  For good measure I'm adding a screenshot of the two different resulting file sets between zlib-chromium and zlib-ng.

Also just a heads up, this change does seem to slightly hurt compression times.  I fully intend to investigate why and mitigate it this weekend.

When I incorporated the Intel inflate changes, we kept a special inflate_fast that was used for inflateBack. Perhaps we can easily switch to that inflate_fast variant depending on whether or not it is using crc32 or not. Or as you said we can come up with a method that works for both ways.

When I incorporated the Intel inflate changes, we kept a special inflate_fast that was used for inflateBack. Perhaps we can easily switch to that inflate_fast variant depending on whether or not it is using crc32 or not. Or as you said we can come up with a method that works for both ways.

Excellent, that makes testing much much easier.  Do you know if there's an easy way to tell if it's requiring a CRC (ala gzip headers) versus adler?

I think I'm mostly off the hook for the deflate performance - the difference must be either coincidence or just pure luck in the function alignment / offsets because these copy functions aren't called anywhere on any of the deflate paths.

@KungFuJesus the detection algorithm for determine that can be found in inf_crc_copy. You might be able to simply add inflateBack to your png_decode algorithm to see how it compares. You just have to add the callback that copies to memory and it should be near equivalent..

@KungFuJesus Compression ratio for our level 1-8 will often be worse than stock zlib, because we don't check for 3-byte matches, only 4-byte and larger. That improved performance a lot (approx 2x compression speed), and it also avoids the whole problem that 3-byte matches too far back in the window can end up taking more space than just using literals.
Level 1 is of course also using deflate_quick that is compressing much worse due to being super simple and very fast.
Why deflate-medium (levels 3-6) is doing so much worse than deflate-slow on that file I don't know, but that is certainly interesting and something that could be interesting to look into whether deflate_medium could be improved in that regard. Alternatively we could switch level 6 (default level) back to deflate_slow in order to avoid that pitfall.
Another important difference in our compression is that we don't use a hash chain like stock zlib uses, jury is out on whether this is good or bad, but it allows us to for example use crc32 intrinsics to more quickly build a hash table.
I agree with your suggestion that we could make AVX require AVX2 cpus, for functions where that would make sense at least (might be all of them, idk). Early AVX was a bit of an unpredictable mess, implementation-wise.

@KungFuJesus the PR for the inflate refactor is PR #1033. My initial benchmarks showed 18% improvement. It may be possible to have a adler32+copy function as you stated and regain any losses..

This also sort of reveals that maybe we'll regain 2% of the CPU cycles by inlining the copy with the adler checksum?  Though, that says nothing about the cache effects the benefits there, that's only cycles saved on the moves most likely.

Just a thought, and I hadn't looked too closely at this but...we are touch the same or adjacent areas in memory with this memmove. I don't suppose that we could maybe break that up until two memmoves and move things properly into the window position in the folded checksum and copy (once we have on in place, that is)?

You know one thing I never tested at the time was how much of the benefit we saw was from using a better crc32 function and how much was from cache locality. Because if all the benefit was from using that crc32 function then could revert the rest.

I'm looking back at that PR and it may be possible to create some variable s->pending that points to the strm->next_out in the instance where it is using adler32 and points to s->window + s->whave + s->wnext in the case of crc32+pclmulqdq.
But I think the cache locality only gives us about ~2.67% gain if we look at @Dead2's aarch64 benchmarks.

Why not just write the incoming bytes into a shifted offset from the newly expired window?

I'm not sure I understand what you mean. You would still have to shift back all of the expired window to write to the new offset?

I know it's trying to be contiguous but I can't see any very apparent reason why the window contents need to be contiguous other than maybe some better cache behavior.

Yes I think that's the reason, but also it is easier to copy matches if it is contiguous?

From my comment on that PR https://github.com/zlib-ng/zlib-ng/pull/1033#issuecomment-971670194

I'm surprised on ARM the gains are not more. Perhaps it is the crc folding that really makes the difference.

I suspect part of the reason why things need to remain a contiguous sliding buffer is that there's some index tracking that relies on it - so whatever change that was made so that the buffers could remain where they are would require some elaborate index tracking to point to the proper positions in the buffer.
I don't know for sure because I don't entirely understand how the inflation window works, but I imagine there's a way that instead of sliding the contents of the buffer out you slide the index positions you're tracking.  When it's time to roll off an already decompressed or expired window of the buffer (like I believe the memmove is doing there), then that are then becomes a contiguous region of free space that can be written back to.  In any case, 32k or 64k for this buffer can fit very well into cache to begin with.  Making things entirely contiguous just makes things easier on the hardware prefetcher.
As far as I can tell, though, when dealing with these regions of memory in the window large heaps of memory are read at a time, anyway.  At least, that what it seems like might be happening when things advance to "wnext".  Of course maybe the original code operated in a similar manner to this.  It seems like zlib-chromium is operating this way and they have an accelerated CRC32 and they are about twice as fast as us for these PNG decodes for the same imagery.
Of course, if that were entirely true I wouldn't think you'd be measuring a gain in performance at all from the cache locality, it would seem like it's a net loss (though, perhaps the examples in my benchmark aren't full representative of all datasets and this is just a worst case scenario).

I'm looking back at that PR and it may be possible to create some variable s->pending that points to the strm->next_out in the instance where it is using adler32 and points to s->window + s->whave + s->wnext in the case of crc32+pclmulqdq.

Definitely seems worth pursuing if you can figure out a way that maybe preserves some cache friendliness but also avoids moving bytes for sliding the window.
For sure the folded copy inside the checksum is valuable.  It seems like that part doesn't actually depend on the reordering of the inflate window layout from the PR.  That part is also extremely easy to implement for the adler checksum.

I did a quite commit to show what I mean:
https://github.com/nmoinvaz/zlib-ng/commit/e829aa49567d81b123d47a0b616f6f10cfb1068a
I wonder if we would get the same cache locality benefits if pending buffer and window were two separate memory allocations because we could just do a swap of the pointers.

I did a quick commit to show what I mean: nmoinvaz@e829aa4
I wonder if we would get the same cache locality benefits if pending buffer and window were two separate memory allocations because we could just do a swap of the pointers.

Hmm, that patch didn't make things meaningfully faster (the memmove is still there and the bytes are still being slid backward in the buffer).

So, with me still not knowing when and how this window gets populated, this is my naive idea that would probably work fairly well and require minimal rework.
We keep the organization of the inflation window the same - things are contiguous for a pretty big span of the 32k buffer.  As the buffer gets consumed, instead of sliding the contents back, we make it behave more like a circular buffer. You move a cursor pointer to it forward.  Then, when new items are populated into the window, you use a write cursor to indicate where in the buffer they go.  Then, you just have to handle weird wrap around effect that only happen every 32k.  So, if something spans a buffer at the wrap around, you do a partial write of the buffer for the first half at the end, then the second half you write into the beginning of the buffer (easy to figure out based on a "remaining bytes", and since 32k is a power of two, the index to write to is also easy to compute with modulus.
When doing a read that spans the split halves of a buffer (which shouldn't be nearly as frequent), you simply leverage a temporary buffer that concatenates both halves for your processing.
I've done similar circular buffering for other things, and it can be tricky to track the indexing properly but this should be fairly feasible.  Of course, I don't know how the corner cases or exactly how this sliding window is used so I'm not the best candidate to implement this.

Of course, none of this PR depends on any of this :).  We should merge this as soon as possible - it fixes a potential bug we were lucky enough to not see and it increases performance drastically.
@nmoinvaz do you see how the circular buffer implementation would work?  Are you comfortable attempting to implement it.  If not, I could stub out some functions that provided you with an interface that abstracted most of that detail away.

I am interested to see some benchmarks on this PR.
I went looking in the code today and I noticed some comments in DFLTCC about a circular history buffer. Also there is a comment in inflate code about a wrap around chunkcopy. I haven't looked more closely into it.
I don't mind if you want to take the lead on it. I have been having a hard time focusing for long periods of time.
There are some other changes I have been wanting to do but I have been waiting on some of the PR to get merged first. @Dead2 is the only one who merges the PRs.

I am interested to see some benchmarks on this PR.

Very well:
Develop (silesia.tar)
 Tool: minideflate Levels: 0-9
 Runs: 30         Trim worst: 3

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.015%      0.046/0.056/0.062/0.004        0.062/0.075/0.082/0.005      211,990,111
 1     44.407%      1.052/1.066/1.075/0.006        0.570/0.585/0.598/0.008       94,124,037
 2     35.518%      1.811/1.825/1.837/0.007        0.602/0.609/0.621/0.005       75,282,949
 3     33.882%      2.192/2.209/2.224/0.008        0.569/0.585/0.595/0.006       71,816,466
 4     33.174%      2.557/2.569/2.577/0.006        0.553/0.567/0.582/0.006       70,315,656
 5     32.672%      2.828/2.841/2.853/0.006        0.552/0.563/0.575/0.006       69,249,937
 6     32.517%      3.401/3.417/3.434/0.009        0.539/0.555/0.565/0.007       68,921,480
 7     32.255%      5.019/5.039/5.055/0.009        0.545/0.558/0.567/0.006       68,366,788
 8     32.167%      7.812/7.840/7.855/0.010        0.536/0.556/0.566/0.008       68,180,764
 9     31.887%      8.144/8.169/8.181/0.009        0.538/0.549/0.559/0.005       67,586,430

 avg1  40.849%                        3.503                          0.520
 avg2  45.388%                        3.892                          0.578
 tot                                945.803                        140.430      865,834,618

PR 1173  (silesia.tar)
 Tool: minideflate Levels: 0-9
 Runs: 30         Trim worst: 3

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.015%      0.044/0.053/0.059/0.004        0.062/0.068/0.076/0.004      211,990,111
 1     44.407%      1.016/1.031/1.043/0.008        0.581/0.596/0.609/0.008       94,124,037
 2     35.518%      1.796/1.811/1.822/0.008        0.599/0.618/0.630/0.008       75,282,949
 3     33.882%      2.212/2.230/2.243/0.008        0.582/0.599/0.610/0.007       71,816,466
 4     33.174%      2.565/2.584/2.597/0.008        0.559/0.579/0.589/0.007       70,315,656
 5     32.672%      2.821/2.847/2.861/0.008        0.556/0.571/0.582/0.007       69,249,937
 6     32.517%      3.409/3.428/3.436/0.008        0.554/0.568/0.578/0.007       68,921,480
 7     32.255%      5.020/5.042/5.055/0.008        0.548/0.562/0.574/0.008       68,366,788
 8     32.167%      7.833/7.851/7.866/0.008        0.544/0.562/0.571/0.008       68,180,764
 9     31.887%      8.150/8.170/8.194/0.011        0.543/0.565/0.573/0.008       67,586,430

 avg1  40.849%                        3.505                          0.529
 avg2  45.388%                        3.894                          0.587
 tot                                946.230                        142.738      865,834,618

What's interesting about this in general is the fact that inflating deflate streams is slower in general, perhaps lending credence to the checksum + copy folding.  Also interesting is the fact that the canonical dataset we're using does not lend itself nearly the compressibility of my benchmark examples.  So, I thought'd I'd give that a whirl.  I've compressed a clean checkout of our source code, tar'd it, and ran it through the same battery.  Plain text is not an unrealistic test, as apache compressing HTTP traffic can happen.  Or, a simple PNG with a limited color palette can have some extreme compressibility.
Develop (zlib-ng-clean.tar)
Tool: minideflate Levels: 0-9
 Runs: 30         Trim worst: 3

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.015%      0.000/0.003/0.005/0.001        0.002/0.004/0.006/0.001       10,886,791
 1     86.390%      0.113/0.118/0.121/0.002        0.031/0.036/0.040/0.002        9,403,620
 2     80.691%      0.175/0.179/0.181/0.002        0.022/0.024/0.027/0.002        8,783,319
 3     80.258%      0.199/0.202/0.205/0.002        0.022/0.026/0.028/0.002        8,736,144
 4     80.062%      0.201/0.206/0.210/0.003        0.021/0.025/0.028/0.002        8,714,821
 5     79.928%      0.211/0.215/0.219/0.002        0.021/0.025/0.027/0.002        8,700,310
 6     79.898%      0.218/0.222/0.225/0.002        0.021/0.024/0.027/0.002        8,696,970
 7     79.853%      0.230/0.234/0.237/0.002        0.021/0.026/0.028/0.002        8,692,114
 8     79.831%      0.261/0.264/0.266/0.001        0.021/0.025/0.027/0.002        8,689,666
 9     79.804%      0.291/0.296/0.298/0.002        0.019/0.024/0.027/0.002        8,686,771

 avg1  82.673%                        0.194                          0.024
 avg2  91.859%                        0.215                          0.027
 tot                                 52.332                          6.467       89,990,526

PR 1173 (zlib-ng-clean.tar)
 Tool: minideflate Levels: 0-9
 Runs: 30         Trim worst: 3

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.015%      0.000/0.002/0.004/0.001        0.001/0.003/0.005/0.001       10,886,791
 1     86.390%      0.107/0.111/0.113/0.002        0.032/0.037/0.040/0.002        9,403,620
 2     80.691%      0.180/0.184/0.186/0.002        0.021/0.025/0.028/0.002        8,783,319
 3     80.258%      0.202/0.208/0.212/0.003        0.019/0.024/0.028/0.002        8,736,144
 4     80.062%      0.209/0.213/0.215/0.002        0.020/0.025/0.028/0.002        8,714,821
 5     79.928%      0.217/0.221/0.224/0.002        0.020/0.024/0.026/0.001        8,700,310
 6     79.898%      0.223/0.227/0.231/0.002        0.020/0.025/0.027/0.002        8,696,970
 7     79.853%      0.238/0.241/0.244/0.002        0.021/0.025/0.028/0.002        8,692,114
 8     79.831%      0.265/0.270/0.273/0.002        0.021/0.025/0.028/0.002        8,689,666
 9     79.804%      0.297/0.300/0.305/0.002        0.019/0.024/0.027/0.002        8,686,771

 avg1  82.673%                        0.198                          0.024
 avg2  91.859%                        0.220                          0.026
 tot                                 53.353                          6.394       89,990,526

Not quite satisfied with the showing that gave as far as compressibility (~20% isn't much), I decided to pull a log file off of my server since this would probably get closer to the compression levels in my png encode demo (1MB -> 4k).
Develop (bmc.log)
 Tool: minideflate Levels: 0-9
 Runs: 30         Trim worst: 3

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.015%      0.002/0.004/0.006/0.001        0.002/0.005/0.008/0.002       14,942,000
 1      9.202%      0.011/0.014/0.016/0.002        0.008/0.013/0.016/0.002        1,374,724
 2      6.814%      0.019/0.022/0.024/0.001        0.009/0.012/0.014/0.001        1,018,064
 3      5.580%      0.033/0.038/0.041/0.002        0.008/0.011/0.014/0.001          833,590
 4      5.396%      0.035/0.041/0.044/0.002        0.009/0.011/0.015/0.002          806,130
 5      5.376%      0.048/0.053/0.058/0.003        0.008/0.011/0.013/0.001          803,183
 6      5.375%      0.104/0.107/0.110/0.002        0.008/0.011/0.014/0.002          803,070
 7      5.778%      0.106/0.108/0.111/0.001        0.008/0.011/0.014/0.002          863,189
 8      5.327%      0.224/0.228/0.231/0.002        0.006/0.011/0.013/0.002          795,775
 9      5.328%      0.282/0.287/0.290/0.002        0.009/0.011/0.013/0.001          796,045

 avg1  15.419%                        0.090                          0.011
 avg2  17.132%                        0.100                          0.012
 tot                                 24.366                          2.918       23,035,770

PR 1173
 Tool: minideflate Levels: 0-9
 Runs: 30         Trim worst: 3

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.015%      0.001/0.004/0.005/0.001        0.002/0.005/0.007/0.001       14,942,000
 1      9.202%      0.012/0.015/0.017/0.001        0.008/0.013/0.016/0.002        1,374,724
 2      6.814%      0.017/0.021/0.023/0.002        0.010/0.013/0.015/0.002        1,018,064
 3      5.580%      0.039/0.042/0.045/0.002        0.008/0.012/0.014/0.002          833,590
 4      5.396%      0.041/0.045/0.047/0.002        0.007/0.011/0.014/0.002          806,130
 5      5.376%      0.054/0.057/0.060/0.002        0.007/0.011/0.014/0.002          803,183
 6      5.375%      0.108/0.111/0.116/0.002        0.008/0.011/0.014/0.001          803,070
 7      5.778%      0.109/0.111/0.113/0.001        0.006/0.011/0.013/0.002          863,189
 8      5.327%      0.225/0.230/0.232/0.002        0.007/0.011/0.013/0.002          795,775
 9      5.328%      0.284/0.289/0.292/0.002        0.007/0.010/0.012/0.001          796,045

 avg1  15.419%                        0.093                          0.011
 avg2  17.132%                        0.103                          0.012
 tot                                 24.986                          2.887       23,035,770

Fascinatingly, the delta is still not as apparent.  Which leads me to believe that for the case of encoding a PNG file, the use of the standard memcpy and the faster memsets has possibly other ancillary benefits that are happening.  We might be fitting into cache more often.  What I don't love about the deflatebench test to begin with is it launches a new binary per compression level, so you incur the startup cost of loading an ELF into your address space each and every run which has high variability and knock-on effects.  That goes without saying we have to setup new TLS each time and repopulate the function table to new virtual addresses.
The differences are very apparent when running the benchmarks included with #1170.  For both a realistic PNG and the R, G, & B striped PNG.  The examples themselves are fully loaded into memory too, so we're not measuring much in the way of system call overhead like we are with minigzip/mindeflate.
The fact that minigzip compressed files are more quickly expanded than deflate compressed ones, even at nearly the same compression ratios, is fascinating.  The adler checksum itself was made to be a faster alternative than a full blown CRC32 and as a result, ought to be faster.  And that is very much true by our own measurements, even with accelerated CRCs:
adler32/avx512_vnni/4096          57.5 ns         57.5 ns     12159999
adler32/avx512_vnni/32768          435 ns          435 ns      1610518
adler32/avx512_vnni/262144        3496 ns         3496 ns       200213
adler32/avx512_vnni/2097152      77536 ns        77536 ns         9024
adler32/avx512_vnni/4194304     155408 ns       155407 ns         4504
crc32/pclmulqdq/4096               174 ns          174 ns      4012515
crc32/pclmulqdq/32768             1273 ns         1273 ns       550034
crc32/pclmulqdq/262144           10029 ns        10029 ns        69796
crc32/pclmulqdq/2097152         106605 ns       106605 ns         6788
crc32/pclmulqdq/4194304         214614 ns       214613 ns         3279

One thing that PNG compression is doing differently is it's including both these checksums, where the CRC32 is run externally on the post-deflated dated + some other header fields.

So I guess the conclusion is, there are gains that aren't being captured very well by the deflatebench benchmark that we'll soon be able to capture.  Sometimes there are losses (possibly if the compression produces small copy lengths).  Minigzip always being faster doesn't quite make that much sense - it could be that we're hotter in cache thanks to the folded checksum but I suspect it could be other things we could find with more detailed profiles.  From what I can gather, minigzip has received quite a bit more attention than minigzip has so minideflate having some bottlenecks isn't outside of the realm of possibilities.
At the very least, this addresses a potential copy hazard encountered in the "chunkcopy_safe" family of functions that doesn't behave in the manner it's supposed to (it's supposed to behave like a self modifying copy loop).  It's ultimately up to @Dead2 but frankly I'd be happier with this patch integrated for the reason of the bug alone, let alone the performance uplift I saw with decoding PNGs.
For all of these benchmarks, I think we can safely disregard compression times, as none of the deflate path actually invokes these copy functions whatsoever.

Not quite content with that, I also ran this with this image:
https://upload.wikimedia.org/wikipedia/commons/6/6a/PNG_Test.png
Ran at all the respective compression levels, just to make sure this wasn't some crazy unrealistic experiment that can't be reproduced with real imagery.  Granted, the times are a lot higher mainly due to the fact that re-exporting the image in gimp doesn't allow me to tell libpng to not apply a "compression filter", so it's doing additional transforms of the data after decompressing it.
PR #1170
--------------------------------------------------------------------------------------
Benchmark                                            Time             CPU   Iterations
--------------------------------------------------------------------------------------
png_decode_realistic/png_decode_realistic/0      18263 us        18263 us           38
png_decode_realistic/png_decode_realistic/1      26249 us        26248 us           27
png_decode_realistic/png_decode_realistic/2      22571 us        22571 us           31
png_decode_realistic/png_decode_realistic/3      20388 us        20387 us           34
png_decode_realistic/png_decode_realistic/4      24450 us        24449 us           29
png_decode_realistic/png_decode_realistic/5      23187 us        23186 us           30
png_decode_realistic/png_decode_realistic/6      22177 us        22176 us           31
png_decode_realistic/png_decode_realistic/7      21990 us        21990 us           32
png_decode_realistic/png_decode_realistic/8      21630 us        21630 us           32
png_decode_realistic/png_decode_realistic/9      21151 us        21151 us           33

PR #1170 + PR #1173
--------------------------------------------------------------------------------------
Benchmark                                            Time             CPU   Iterations
--------------------------------------------------------------------------------------
png_decode_realistic/png_decode_realistic/0      18200 us        18199 us           38
png_decode_realistic/png_decode_realistic/1      27381 us        27380 us           25
png_decode_realistic/png_decode_realistic/2      22973 us        22972 us           30
png_decode_realistic/png_decode_realistic/3      20412 us        20412 us           34
png_decode_realistic/png_decode_realistic/4      24252 us        24251 us           29
png_decode_realistic/png_decode_realistic/5      23035 us        23035 us           30
png_decode_realistic/png_decode_realistic/6      22093 us        22093 us           32
png_decode_realistic/png_decode_realistic/7      21907 us        21906 us           32
png_decode_realistic/png_decode_realistic/8      21659 us        21659 us           32
png_decode_realistic/png_decode_realistic/9      21297 us        21297 us           33

So the effects clearly aren't nearly as pronounced when the compression ratios aren't nearly as high.  Then again, we're crawling all around that data because we're including some additional data transforms imposed by libpng that aren't there when we don't enable these.  If the knock-on warmer cache effects are a nice side effect of this, we could very well be nullifying that with PNG's additional transforms.  A simpler graphic could still have much extreme benefits with this PR.  Compression sizes:
--------------------------------------------------------------------------------------
Benchmark                                            Time             CPU   Iterations
--------------------------------------------------------------------------------------
png_decode_realistic/png_decode_realistic/0      18200 us        18199 us           38
png_decode_realistic/png_decode_realistic/1      27381 us        27380 us           25
png_decode_realistic/png_decode_realistic/2      22973 us        22972 us           30
png_decode_realistic/png_decode_realistic/3      20412 us        20412 us           34
png_decode_realistic/png_decode_realistic/4      24252 us        24251 us           29
png_decode_realistic/png_decode_realistic/5      23035 us        23035 us           30
png_decode_realistic/png_decode_realistic/6      22093 us        22093 us           32
png_decode_realistic/png_decode_realistic/7      21907 us        21906 us           32
png_decode_realistic/png_decode_realistic/8      21659 us        21659 us           32
png_decode_realistic/png_decode_realistic/9      21297 us        21297 us           33

Also interestingly, for that particular test image, we are beating zlib-chromium, by somewhat narrow margins some of the time:
--------------------------------------------------------------------------------------
Benchmark                                            Time             CPU   Iterations
--------------------------------------------------------------------------------------
png_decode_realistic/png_decode_realistic/0       8961 us         8961 us           77
png_decode_realistic/png_decode_realistic/1      21683 us        21682 us           32
png_decode_realistic/png_decode_realistic/2      21035 us        21035 us           33
png_decode_realistic/png_decode_realistic/3      20406 us        20405 us           34
png_decode_realistic/png_decode_realistic/4      21915 us        21914 us           32
png_decode_realistic/png_decode_realistic/5      21398 us        21398 us           33
png_decode_realistic/png_decode_realistic/6      20805 us        20804 us           33
png_decode_realistic/png_decode_realistic/7      20701 us        20701 us           34
png_decode_realistic/png_decode_realistic/8      20512 us        20512 us           34
png_decode_realistic/png_decode_realistic/9      20216 us        20216 us           34

In that circumstance, around 8% of our time is still in memmove, so the optimization whereby I slide a pointer in the windows instead of the actual window, is going to pay off.  As will eliding the copy + checksum.

X64-64
baseline
 Tool: minigzip   Levels: 0-9
 Runs: 75         Trim worst: 42

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.008%      0.010/0.014/0.018/0.003        0.013/0.017/0.020/0.002       15,737,543
 1     54.166%      0.205/0.219/0.226/0.005        0.099/0.112/0.118/0.005        8,523,732
 2     43.871%      0.414/0.429/0.435/0.005        0.113/0.119/0.123/0.003        6,903,609
 3     42.387%      0.526/0.536/0.542/0.004        0.110/0.114/0.120/0.003        6,670,099
 4     41.647%      0.571/0.590/0.595/0.005        0.106/0.112/0.116/0.003        6,553,723
 5     41.216%      0.652/0.665/0.671/0.005        0.097/0.109/0.115/0.005        6,485,938
 6     41.037%      0.739/0.751/0.757/0.005        0.093/0.109/0.113/0.005        6,457,776
 7     40.778%      0.915/0.939/0.947/0.007        0.092/0.106/0.112/0.005        6,416,919
 8     40.704%      1.140/1.154/1.160/0.005        0.098/0.108/0.112/0.004        6,405,244
 9     40.409%      1.362/1.371/1.378/0.005        0.099/0.108/0.115/0.005        6,358,951

 avg1  48.622%                        0.667                          0.101
 avg2  54.025%                        0.741                          0.113
 tot                                220.052                         33.486       76,513,534

   text    data     bss     dec     hex filename
 132309    1464      48  133821   20abd libz-ng.so.2

PR #1173
 Tool: minigzip   Levels: 0-9
 Runs: 75         Trim worst: 42

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.008%      0.006/0.014/0.017/0.003        0.013/0.018/0.022/0.002       15,737,543
 1     54.166%      0.204/0.219/0.225/0.006        0.109/0.114/0.119/0.003        8,523,732
 2     43.871%      0.419/0.430/0.436/0.004        0.104/0.118/0.124/0.006        6,903,609
 3     42.387%      0.518/0.535/0.542/0.005        0.105/0.118/0.124/0.004        6,670,099
 4     41.647%      0.576/0.592/0.597/0.005        0.093/0.111/0.120/0.007        6,553,723
 5     41.216%      0.641/0.653/0.659/0.005        0.095/0.109/0.115/0.005        6,485,938
 6     41.037%      0.730/0.740/0.748/0.005        0.084/0.106/0.114/0.008        6,457,776
 7     40.778%      0.925/0.936/0.943/0.005        0.096/0.106/0.113/0.005        6,416,919
 8     40.704%      1.138/1.149/1.157/0.005        0.100/0.107/0.113/0.004        6,405,244
 9     40.409%      1.368/1.378/1.384/0.005        0.100/0.112/0.117/0.004        6,358,951

 avg1  48.622%                        0.665                          0.102
 avg2  54.025%                        0.739                          0.113
 tot                                219.337                         33.617       76,513,534

   text    data     bss     dec     hex filename
 134069    1456      48  135573   21195 libz-ng.so.2

ARM
baseline
 Tool: minigzip   Levels: 0-9
 Runs: 10         Trim worst: 6

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.008%      0.170/0.195/0.210/0.017        0.190/0.215/0.250/0.030       15,737,543
 1     54.185%      3.430/3.482/3.530/0.046        0.830/0.837/0.850/0.010        8,526,745
 2     43.874%      5.060/5.077/5.090/0.013        0.790/0.800/0.820/0.014        6,904,168
 3     42.394%      6.100/6.168/6.220/0.051        0.750/0.778/0.790/0.019        6,671,197
 4     41.693%      7.000/7.050/7.080/0.036        0.800/0.800/0.800/0.000        6,560,888
 5     41.216%      7.570/7.625/7.690/0.055        0.770/0.778/0.790/0.010        6,485,936
 6     41.038%      9.040/9.070/9.120/0.036        0.750/0.762/0.770/0.010        6,457,827
 7     40.778%   11.070/11.255/11.430/0.172        0.690/0.742/0.790/0.041        6,416,941
 8     40.704%   13.570/13.765/13.910/0.160        0.660/0.710/0.740/0.035        6,405,249
 9     40.409%   13.050/13.143/13.250/0.102        0.740/0.775/0.810/0.040        6,358,951

 avg1  48.630%                        7.683                          0.720
 avg2  54.033%                        8.537                          0.800
 tot                                307.320                         28.790       76,525,445

   text    data     bss     dec     hex filename
 105398     856      16  106270   19f1e libz-ng.so.2

PR #1173
 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.008%      0.180/0.195/0.200/0.010        0.190/0.217/0.250/0.028       15,737,543
 1     54.185%      3.400/3.473/3.510/0.052        0.780/0.802/0.810/0.015        8,526,745
 2     43.874%      5.000/5.050/5.080/0.035        0.800/0.820/0.830/0.014        6,904,168
 3     42.394%      6.200/6.252/6.310/0.048        0.770/0.782/0.790/0.010        6,671,197
 4     41.693%      6.920/7.080/7.150/0.108        0.760/0.780/0.800/0.018        6,560,888
 5     41.216%      7.690/7.737/7.770/0.034        0.750/0.760/0.770/0.012        6,485,936
 6     41.038%      8.890/8.913/8.960/0.032        0.710/0.732/0.760/0.026        6,457,827
 7     40.778%   11.240/11.265/11.280/0.019        0.710/0.735/0.770/0.026        6,416,941
 8     40.704%   13.490/13.700/13.870/0.185        0.700/0.740/0.770/0.029        6,405,249
 9     40.409%   12.980/13.098/13.160/0.085        0.740/0.768/0.780/0.019        6,358,951

 avg1  48.630%                        7.676                          0.714
 avg2  54.033%                        8.529                          0.793
 tot                                307.050                         28.550       76,525,445

   text    data     bss     dec     hex filename
 107422     844      16  108282   1a6fa libz-ng.so.2

AARCH64
baseline
 Tool: minigzip   Levels: 0-9
 Runs: 40         Trim worst: 22

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.008%      0.008/0.016/0.020/0.004        0.024/0.034/0.041/0.006       15,737,543
 1     54.165%      0.538/0.544/0.551/0.004        0.183/0.203/0.215/0.011        8,523,602
 2     43.877%      0.895/0.921/0.937/0.014        0.192/0.203/0.212/0.007        6,904,649
 3     42.389%      1.207/1.228/1.244/0.011        0.168/0.191/0.204/0.011        6,670,476
 4     41.647%      1.338/1.374/1.384/0.011        0.179/0.192/0.199/0.005        6,553,763
 5     41.217%      1.491/1.507/1.515/0.007        0.175/0.187/0.193/0.005        6,485,981
 6     41.037%      1.726/1.756/1.768/0.012        0.167/0.183/0.196/0.009        6,457,780
 7     40.778%      2.408/2.430/2.442/0.010        0.157/0.175/0.186/0.009        6,416,924
 8     40.704%      2.922/2.953/2.964/0.010        0.156/0.176/0.191/0.012        6,405,244
 9     40.409%      3.307/3.340/3.353/0.014        0.181/0.192/0.202/0.007        6,358,951

 avg1  48.623%                        1.607                          0.174
 avg2  54.026%                        1.785                          0.193
 tot                                289.227                         31.257       76,514,913

   text    data     bss     dec     hex filename
 116734    1664      16  118414   1ce8e libz-ng.so.2

PR #1173
 Tool: minigzip   Levels: 0-9
 Runs: 40         Trim worst: 22

 Level   Comp   Comptime min/avg/max/stddev  Decomptime min/avg/max/stddev  Compressed size
 0    100.008%      0.004/0.014/0.020/0.005        0.032/0.037/0.044/0.003       15,737,543
 1     54.165%      0.523/0.545/0.552/0.007        0.189/0.199/0.210/0.007        8,523,602
 2     43.877%      0.890/0.920/0.934/0.012        0.193/0.205/0.213/0.007        6,904,649
 3     42.389%      1.199/1.224/1.238/0.012        0.176/0.196/0.206/0.009        6,670,476
 4     41.647%      1.353/1.373/1.381/0.007        0.178/0.192/0.204/0.008        6,553,763
 5     41.217%      1.489/1.508/1.518/0.009        0.184/0.190/0.198/0.004        6,485,981
 6     41.037%      1.736/1.755/1.767/0.011        0.169/0.188/0.197/0.008        6,457,780
 7     40.778%      2.395/2.422/2.434/0.010        0.169/0.183/0.193/0.008        6,416,924
 8     40.704%      2.935/2.950/2.961/0.008        0.153/0.182/0.207/0.016        6,405,244
 9     40.409%      3.327/3.345/3.359/0.011        0.174/0.190/0.204/0.007        6,358,951

 avg1  48.623%                        1.606                          0.176
 avg2  54.026%                        1.784                          0.196
 tot                                289.030                         31.727       76,514,913

   text    data     bss     dec     hex filename
 116990    1640      16  118646   1cf76 libz-ng.so.2

My tests show no significant performance changes either way, but also no ill effects.

Admittedly the performance differences are pretty data dependent.  But when they do make a difference, it seems pretty substantial.


100769 - [Newsroom] Newsfeed - Incorrect alignment for date range and calendar year filter in filter news section in extra small devices.

Severity - 3 - Medium
Steps to Reproduce:
In the newsfeed, On clicking the arrow beside filter news the calendar year filter calendar alignment is incorrect in extra small devices.(iPhone se).
Migrated page: https://main--accenture-newsroom--hlxsites.hlx.live/subjects/analytics
Legacy page: https://newsroom.accenture.com/subjects/analytics
Expected:

Actual:

Azdo link - https://dev.azure.com/accenturecio26/WebContentDigAssetMgmt_PR1093/_workitems/edit/100769
CC: @ravkiran @amol-anand @raymond-h-dev

hi @jjjaspher @veejayricana @zozo-j  - can you verify with the team if this still a valid bug?


Fixed where clause for MariaDB compatibility

Not gonna merge this unless you've tested with MySQL, Postgres, SQL Server, SQlite... and benchmarked performance. It's a critical query in the database notifications feature

Mmmh, I first thought the fix was easy because the -> operator is just an alias/shortcut for JSON_EXTRACT in MySQL
(https://dev.mysql.com/doc/refman/8.0/en/json-search-functions.html#operator_json-column-path) so there would be no performance difference. Also, a similar PR was accepted in the past
https://github.com/filamentphp/filament/pull/2524
But it is true that json_extract() is not available on Postgres.
So it needs a condition similar to this:
https://github.com/filamentphp/filament/blob/6e6d21e8670e65e09dada16a7222c320605178ec/packages/tables/src/Columns/Concerns/InteractsWithTableQuery.php#L103C21-L103C21
Anyway in the current state, would be useful to mention in the docs that MariaDB is not fully supported.

Can we tell if a connection is MariaDB or a different database based on the Connection object from Laravel?

Unfortunately, $query->getConnection()->getDriverName() also returns 'mysql', but $query->getConnection()->isMaria() works.
The method is from Illuminate\Database\MysqlConnection so I added an additional condition to avoid calling isMaria() on PostgresConnection, SQLiteConnection or SqlServerConnection.

Looks good. Thanks

I’m not sure am I missing something here. I definitely been using where(‘column->key’, ‘abc’) and it works.
And eloquent have this exact handling for mysql/mariadb here https://github.com/illuminate/database/blob/c29a5c3719c9ac7e0909d54cdcf13aa22cc666b4/Query/Grammars/MySqlGrammar.php#L366C40-L366C40
are we changing this for the sake of changing or @kaktusss there is an actual error you hitting?

@wychoong
MariaDB doesn't support the json arrow search notation:
https://laravel.com/docs/10.x/queries#json-where-clauses
https://jira.mariadb.org/browse/MDEV-13594
The query doesn't trigger an error on my MariaDB instance, it just doesn't find any result because in can't extract with the -> operator.

The query doesn't trigger an error on my MariaDB instance, it just doesn't find any result because it can't extract with the -> operator.

are you sure?

MariaDB doesn't support the json arrow search notation:
https://laravel.com/docs/10.x/queries#json-where-clauses
https://jira.mariadb.org/browse/MDEV-13594

I'm aware of the incompatibilities or mariadb and mysql you linked before. While the Laravel Doc you referenced didn't explicitly mentioned mariadb, it's common to assume it is compatible with it unless specifically mentioned its not. And that's the whole point of Eloquent, to handle database abstraction as much as it can, as you can see in the link I referred earlier
https://github.com/illuminate/database/blob/c29a5c3719c9ac7e0909d54cdcf13aa22cc666b4/Query/Grammars/MySqlGrammar.php#L366-L371
    protected function wrapJsonSelector($value)
    {
        [$field, $path] = $this->wrapJsonFieldAndPath($value);

        return 'json_unquote(json_extract('.$field.$path.'))';
    }

https://github.com/illuminate/database/blob/c29a5c3719c9ac7e0909d54cdcf13aa22cc666b4/Concerns/CompilesJsonPaths.php#L15-L24
    protected function wrapJsonFieldAndPath($column)
    {
        $parts = explode('->', $column, 2);

        $field = $this->wrap($parts[0]);

        $path = count($parts) > 1 ? ', '.$this->wrapJsonPath($parts[1], '->') : '';

        return [$field, $path];
    }

for the sake of testing, i ran this in tinker
> use Filament\Notifications\Notification;
> use Illuminate\Notifications\DatabaseNotification;
> DatabaseNotification::count();
= 0

> $user = User::first();

> Notification::make()->title('Saved successfully')->sendToDatabase($user);

> DatabaseNotification::count();
= 1

> $query = $user->notifications();

> $databaseConnection = $query->getQuery()->getConnection();

> $databaseConnection->isMaria()
= true

> $query->where('data->format', 'filament')->count()
= 1

> DatabaseNotification::count();
= 1

and it works fine on v3.0.39

So again, is there an error in your app that this actually fixes and provide a repo to replicate? If not I did suggest @danharrin to revert this as it is not actually fixing anything

@wychoong
Thanks for the detailed answer. I could reproduce my issue in tinker with the code you provided (0 result).
I had a look at the raw SQL query and you're totally right, it has been transformed to
json_unquote(json_extract(`data`, '$."format"')) = 'filament'
By removing the double quotes from "format", results were then returned.
Then it became obvious that I should try a more recent version of MariaDB (I'm using Laragon). Should have been more obvious before actually...
Updated to 10.6 to have the same version as the Ploi.io deployed version and that's it, now everything works as expected.
I'm sorry for the waste of time and thanks again for clarifying.
As you said, these changes should be reverted.

Okay I will revert

This PR is really weird, sorry.
I'm using MariaDB for years together with Laravel, and I can use something like this just fine:
Book::query()
    ->where('options->color', 'green')

I think something else is wrong, I think the cast is missing (json or array).
If you need more control, something like https://github.com/spatie/laravel-schemaless-attributes would be interesting.
But the same thing remains, I haven't seen any issues with JSON column parsing using MariaDB. If it would, I think you better of reporting this to Laravel itself.

Please see the thread, it is going to be reverted due to it being a legacy MariaDB issue.

It was done days ago

@danharrin Any news about the revert? :) Should I create a ticket for this?
I would like to upgrade Filament, but I'm afraid this breaks notifications, as I'm using the latest MariaDB.
Many thanks

its already done https://github.com/filamentphp/filament/commit/bda777ed11eb9f82a04a4414aaa0a69cc1b81881

Ah sorry, missed that one!
Great for the fast fix. :)


unknown error

Description
What steps will reproduce the problem?

painter.fillRect(event.rect(), self.sideareas_color)
TypeError:   fillRect(self, QRectF, Qt.BrushStyle): argument 1 has unexpected type 'QRect'
Traceback
  File "C:\Users\Sreekar\Anaconda3\lib\site-packages\spyder\widgets\sourcecode\codeeditor.py", line 171, in paintEvent

Traceback (most recent call last):
  File "C:\Users\Sreekar\Anaconda3\lib\site-packages\spyder\widgets\sourcecode\codeeditor.py", line 213, in paintEvent
    self.code_editor.scrollflagarea_paint_event(event)
  File "C:\Users\Sreekar\Anaconda3\lib\site-packages\spyder\widgets\sourcecode\codeeditor.py", line 1445, in scrollflagarea_paint_event
    painter.fillRect(event.rect(), self.sideareas_color)
TypeError:   fillRect(self, QRectF, Qt.BrushStyle): argument 1 has unexpected type 'QRect'
Traceback (most recent call last):
  File "C:\Users\Sreekar\Anaconda3\lib\site-packages\spyder\widgets\sourcecode\codeeditor.py", line 171, in paintEvent
    self.code_editor.linenumberarea_paint_event(event)
  File "C:\Users\Sreekar\Anaconda3\lib\site-packages\spyder\widgets\sourcecode\codeeditor.py", line 1178, in linenumberarea_paint_event
    painter.fillRect(event.rect(), self.sideareas_color)
TypeError:   fillRect(self, QRectF, Qt.BrushStyle): argument 1 has unexpected type 'QRect'

Versions

Spyder version: 3.3.2
Python version: 3.7.1
Qt version: 5.9.6
PyQt5 version: 5.9.2
Operating System: Windows 10

Dependencies
pyflakes >=0.6.0  :  2.0.0 (OK)
pycodestyle >=2.3 :  2.4.0 (OK)
pygments >=2.0    :  2.3.1 (OK)
sphinx >=0.6.6    :  1.8.2 (OK)
rope >=0.9.4      :  0.11.0 (OK)
jedi >=0.9.0      :  0.13.2 (OK)
nbconvert >=4.0   :  5.4.0 (OK)
pandas >=0.13.1   :  0.23.4 (OK)
numpy >=1.7       :  1.15.4 (OK)
sympy >=0.7.3     :  1.3 (OK)
cython >=0.21     :  0.29.2 (OK)
qtconsole >=4.2.0 :  4.4.3 (OK)
IPython >=4.0     :  7.2.0 (OK)
matplotlib >=2.0.0:  3.0.2 (OK)
pylint >=0.25     :  2.2.2 (OK)

Please describe step by step how you got this error so we can reproduce it in our side. If you fail to do that, we'll close this issue in a week from now.
Also, please read our
https://github.com/spyder-ide/spyder/wiki/Troubleshooting-Guide-and-FAQ
to see if you are able to fix this problem by following it.

Closing due to lack of response.

Mr Cordoba, I have this same issue now, since yesterday after I upgraded python-qtconsole to 5.2.2-1 (last release). What could I provide as information?

@ElMastro, please stop using the Arch or Manjaro packages for Spyder because they are semibroken (i.e. they sort of work but have some issues because they depend on versions we don't support at moment). Besides, as I already told you, Spyder is not ready for Python 3.10, so there are some bugs that also appear due to that.

I was keeping using to keep the package system's consistency. Do you believe using the version of Pypl (pip install spyder) is ok?
Il 28 gen 2022, 04:18 +0100, Carlos Cordoba @.***>, ha scritto:

@ElMastro, please stop using the Arch or Manjaro packages for Spyder because they are semibroken (i.e. they sort of work but have some issues because they depend on versions we don't support at moment). Besides, as I already told you, Spyder is not ready for Python 3.10, so there are some bugs that also appear due to that.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
 Instead, create a pyenv for Python 3.9 first and then install Spyder there.

All right, I understand. Thank you for the suggestion
Il 28 gen 2022, 16:42 +0100, Carlos Cordoba @.***>, ha scritto:

Yes, it is ok, but don't install it with sudo pip. Instead, create a pyenv for Python 3.9 first and then install Spyder there.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.Message ID: @.***>


MAINT: sparse.linalg: A minor improvement with zero initial guess

Reference issue

What does this implement/fix?

Residual norm should be calculated according to the specific value of the initial guess
Additional information

This looks like it is mainly for performance. Is that correct?
(If x0 is None, then the x returned by make_system is a vector of zeroes.)

This looks like it is mainly for performance. Is that correct?
(If x0 is None, then the x returned by make_system is a vector of zeroes.)

Yes. b - Ax does not need to be calculated in the case of the zero initial guess.

This looks like it is mainly for performance. Is that correct?
(If x0 is None, then the x returned by make_system is a vector of zeroes.)

Yes. b - Ax does not need to be calculated in the case of the zero initial guess.


refactor: sensor multilevel constants

This refactors how we determine multilevel sensor types to be similar to how we do alarm sensors. It includes the names for the object ids.
@robertsLando there are some cases where the object id has the same name as the sensor type, so I kept it blank to avoid names like direction_direction. Let me know what you think.
Also, some sensors look like they belong to other types, but I didn't want to move them around just yet. For example, sensor 57 is under sensor_water, but it's a water pressure sensor, and maybe it should be under sensor_pressure instead..
Another thing to consider is in the future we may want to populate not just object ids but also units of measurement based on the constants.
It would be really great if instead of redefining things in the Constants.js if we could just read in the open-zwave xml files that it's based on and parse them to the correct attributes, i.e.: config/Localization.xml and config/SensorMultiLevelCCTypes.xml
Cheers,

I added the missing sensors and changed null to undefined.
As for the other map for the icons, I think it would be better to do that in a separate PR. We could also do something similar for the units of measurement.
The more I look at this the more I think we should just read in the config/NotificationCCTypes.xml and config/SensorMultiLevelCCTypes.xml files and parse them into objects we can use. They have id and name that we can use. Name can be changed to lower case and special characters changed to _. And for the sensor multilevel we could even use the first unit of measurement in the list.
This way we don't have to keep our own maps, and just use what's supplied by OZW. The only map we would need is for the icons.

@chilicheech I understand what you mean but I prefer to keep my own map for now, the best thing would be getting them from OZW directly instead of reading xml files but that requires a change in openzwave-shared library

As for the other map for the icons, I think it would be better to do that in a separate PR. We could also do something similar for the units of measurement.

I prefer to merge this once we clean up all sensors, there is still lot confusion in configuration.js file too many repeating configurations. About units of measurements them are overrided by valueId units so they can all be removed from configurations
Thanks for your help @chilicheech

Thanks. I saw the modifications you made with the props field. Looks good.
Cheers,

Thanks to you! :)
Daniel

On 14 Feb 2020, at 18:47, Thiago Oliveira<EMAIL_ADDRESS>wrote:

Thanks. I saw the modifications you made with the props field. Looks good.
Cheers,
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or unsubscribe.