8000 Linked Blocks compression (-BD) can employ multiple threads by Cyan4973 · Pull Request #1436 · lz4/lz4 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Linked Blocks compression (-BD) can employ multiple threads #1436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jul 10, 2024
Merged

Linked Blocks compression (-BD) can employ multiple threads #1436

merged 4 commits into from
Jul 10, 2024

Conversation

Cyan4973
Copy link
Member
@Cyan4973 Cyan4973 commented Jul 8, 2024 •
edited
Loading

also works with dictionary (-D dictFile).

Note: after this change, keeping a single-threaded version of the compressor in the source code becomes less relevant. All compression modes can now employ the multi-threaded version. This version silently turns into a single-threaded one when LZ4IO_MULTITHREAD==0.
This could be a good reason to reduce the code size.

@Cyan4973
Copy link
Member Author
Cyan4973 commented Jul 8, 2024

There is a CI issue reported for the m68k qemu test,
but it's unclear if this comes from this code base, or if it's a bug in either qemu or the kernel for m68k.

@Cyan4973
Copy link
Member Author
Cyan4973 commented Jul 8, 2024

Given that the m68k architecture is pretty old and quite different from all other platforms, and given that all other emulated and native platforms seem to work fine, the suspicion is on the m68k platform or its emulation.

A work around could be to disable Multithreading when testing with qemu-m68k-static.

@t-mat
Copy link
Contributor
t-mat commented Jul 8, 2024 •
edited
Loading

I've also tested this branch on my environment (Ubuntu 22.04, WSL2).

It ends with same error:

test lz4 qemu-m68k-static platform 
 ---- test lz4 qemu-m68k-static platform ----
qemu-m68k-static ./datagen -g16KB  | qemu-m68k-static ../programs/lz4 -9     | qemu-m68k-static ../programs/lz4 -t

stdin                          : decoded 16384 bytes
qemu-m68k-static ./datagen         | qemu-m68k-static ../programs/lz4        | qemu-m68k-static ../programs/lz4 -t

stdin                          : decoded 65536 bytes
qemu-m68k-static ./datagen -g256MB | qemu-m68k-static ../programs/lz4 -vqB4D | qemu-m68k-static ../programs/lz4 -qt
using blocks of size 64 KB

The futex facility returned an unexpected error code.
...
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted
lz4: lz4io.c:2192: LZ4IO_decompressLZ4F: Assertion `lastStatus == 0' failed.
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted
make[1]: *** [Makefile:310: test-platform] Error 134

But when I split piped commands into separated commands:

  • Compression program (lz4 -vqB4D) aborts
    • But it generates proper .lz4 file
  • Decompression program (lz4 -d) decompress it properly
  • Original file and decompressed file are same.

It may suggest that program/process itself is working correctly but it doesn't terminate something gracefully.

Same test, but splitted 
cd ./tests/

qemu-m68k-static ./datagen -g256MB > data-256MB
# OK

qemu-m68k-static ../programs/lz4 -vqB4D data-256MB
using blocks of size 64 KB
Compressed filename will be : data-256MB.lz4
The futex facility returned an unexpected error code.
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted
# FAILED ?

qemu-m68k-static ../programs/lz4 -d data-256MB.lz4 data-256MB.lz4.dec
data-256MB.lz4                 : decoded 268435456 bytes
# OK

cmp data-256MB data-256MB.lz4.dec
# OK

@Cyan4973
Copy link
Member Author
Cyan4973 commented Jul 8, 2024

Thanks for the analysis @t-mat .
So the whole multithreaded compression works fine,
it's closing a futex, at end of compression, that causes a segfault.

@Cyan4973
disable multithreading when compiling for m68k
7aaf095 
there seems to be a bug somewhere, either qemu or pthread, on this target
@Cyan4973
Copy link
Member Author
Cyan4973 commented Jul 9, 2024

Let's test m68k without multithreading then. It's not a good fit anyway (multi-cores m68k doesn't exist).

@Cyan4973 Cyan4973 merged commit 291c3ae into dev Jul 10, 2024
126 checks passed
@Cyan4973 Cyan4973 deleted the MT_linkedBlocks branch July 21, 2024 04:12
@moonfruit moonfruit mentioned this pull request Jul 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Cyan4973 @t-mat
0