Bug 233543 - bsdtar: enable xz threads support
Summary: bsdtar: enable xz threads support
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.0-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: Martin Matuska
URL:
Keywords: feature, patch, performance
: 234217 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-11-26 18:55 UTC by Stefan Ehmann
Modified: 2024-01-02 20:09 UTC (History)
8 users (show)

See Also:


Attachments
patch enabling xz threads support (533 bytes, patch)
2018-11-26 18:55 UTC, Stefan Ehmann
no flags Details | Diff
patch for ports-mgmt/pkg to fix pkg-static linking (784 bytes, patch)
2019-02-10 22:00 UTC, Stefan Ehmann
no flags Details | Diff
bsdtar(1) patch (745 bytes, patch)
2021-09-28 05:22 UTC, Felix Johnson
no flags Details | Diff
Re-enable multi-threading support for CURRENT (335c4f8edb3a) (1.27 KB, patch)
2021-09-30 06:59 UTC, Jung-uk Kim
no flags Details | Diff
Re-enable multi-threading support for CURRENT (3ec0dc367bff) (1.01 KB, patch)
2022-02-07 21:39 UTC, Jung-uk Kim
no flags Details | Diff
Re-enable multi-threading support for CURRENT after 7f815d4f128f (1.07 KB, patch)
2022-04-08 13:18 UTC, Jung-uk Kim
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Ehmann 2018-11-26 18:55:24 UTC
Created attachment 199578 [details]
patch enabling xz threads support

Currently, bsdtar is built without XZ multi-threading support.

Can be tested with this command:
tar -Jcf /dev/null --options xz:threads=4 $HOME

After setting HAVE_LZMA_STREAM_ENCODER_MT, it works as expected, see attached patch.

If this feature is enabled, the xz:threads option should also be mentioned in the man page.

Either add a reference to -T option in xz(1) or duplicate the info from there:
> Specify the number of worker threads to use. Setting threads to
> a special value 0 makes xz use as many threads as there are CPU
> cores on the system.
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2018-11-26 19:08:03 UTC
+1

Alternatively, a workaround is to just use 'tar -f - | xz -T0'.
Comment 2 commit-hook freebsd_committer freebsd_triage 2018-11-26 21:46:03 UTC
A commit references this bug:

Author: mm
Date: Mon Nov 26 21:45:27 UTC 2018
New revision: 340997
URL: https://svnweb.freebsd.org/changeset/base/340997

Log:
  libarchive configuration changes
  - move HAVE_BZLIB_H, HAVE_LIBLZMA and HAVE_LZMA_H to config_freebsd.h
  - activate support for multi-threaded lzma encoding [1]

  PR:		233543 [1]
  Reported by:	cem
  MFC after:	1 week

Changes:
  head/lib/libarchive/Makefile
  head/lib/libarchive/config_freebsd.h
Comment 3 Conrad Meyer freebsd_committer freebsd_triage 2018-11-26 23:32:36 UTC
(In reply to commit-hook from comment #2)
I didn't report it :-).
Comment 4 Martin Matuska freebsd_committer freebsd_triage 2018-11-27 09:25:44 UTC
Sorry, oversaw that.
Comment 5 Conrad Meyer freebsd_committer freebsd_triage 2018-12-20 16:10:05 UTC
*** Bug 234217 has been marked as a duplicate of this bug. ***
Comment 6 Florian Smeets freebsd_committer freebsd_triage 2018-12-22 05:03:37 UTC
Mention that the patch that was committed in r340997 was reverted in r341453:

Author: sbruno
Date: Tue Dec  4 03:23:14 2018
New Revision: 341453
URL: https://svnweb.freebsd.org/changeset/base/341453

Log:
  Revert r340997 at the request of multiple users.
  - breaks ports-mgmt/pkg build for mips64, powerpc64 and i386 for some users.
  
  --- pkg-static ---
  /usr/lib/liblzma.a(stream_encoder_mt.o): In function `mythread_cond_init':
  /usr/local/poudriere/jails/ppc64/usr/src/contrib/xz/src/common/mythread.h:230:
  undefined reference to `pthread_condattr_init'
  
  Reported by:	jhibbits zeising
Comment 7 Stefan Ehmann 2018-12-22 11:34:32 UTC
I can reproduce this on i386, see below.

The problem is that -llzma occurs before -lpthread on the linker command for pkg-static.
If -lpthread is put at the end of the command, it links OK.
So far, I haven't been able to fix this is in the build system, probably some libtool magic required.

/bin/sh ../libtool  --tag=CC    --mode=link cc  -O2 -pipe  -Wno-error -fstack-protector -fno-strict-aliasing  -Wall -Wno-unused-function -D_BSD_SOURCE -DINET6=1  -all-static -fstack-protector   -Wl,--enable-new-dtags -o pkg-static  pkg-add.o pkg-alias.o  pkg-annotate.o pkg-audit.o  pkg-autoremove.o pkg-backup.o  pkg-check.o pkg-clean.o pkg-config.o  pkg-convert.o pkg-create.o  pkg-delete.o pkg-event.o pkg-fetch.o  pkg-globals.o pkg-info.o pkg-install.o  pkg-lock.o pkg-main.o pkg-plugins.o  pkg-query.o pkg-register.o pkg-repo.o  pkg-rquery.o pkg-search.o pkg-set.o  pkg-shell.o pkg-shlib.o pkg-ssh.o  pkg-stats.o pkg-update.o  pkg-updating.o pkg-upgrade.o  pkg-utils.o pkg-version.o pkg-which.o  ../libpkg/libpkg_static.la  ../compat/libbsd_compat.la -ljail  -larchive -lz -lutil -lbz2 -llzma -lssl  -lpthread -lcrypto -lm  -lelf  -ljail -larchive -lz -lbz2 -llzma
libtool: link: cc -O2 -pipe -Wno-error -fstack-protector -fno-strict-aliasing -Wall -Wno-unused-function -D_BSD_SOURCE -DINET6=1 -static -fstack-protector -Wl,--enable-new-dtags -o pkg-static pkg-add.o pkg-alias.o pkg-annotate.o pkg-audit.o pkg-autoremove.o pkg-backup.o pkg-check.o pkg-clean.o pkg-config.o pkg-convert.o pkg-create.o pkg-delete.o pkg-event.o pkg-fetch.o pkg-globals.o pkg-info.o pkg-install.o pkg-lock.o pkg-main.o pkg-plugins.o pkg-query.o pkg-register.o pkg-repo.o pkg-rquery.o pkg-search.o pkg-set.o pkg-shell.o pkg-shlib.o pkg-ssh.o pkg-stats.o pkg-update.o pkg-updating.o pkg-upgrade.o pkg-utils.o pkg-version.o pkg-which.o  ../libpkg/.libs/libpkg_static.a ../compat/.libs/libbsd_compat.a -lutil -lssl -lpthread -lcrypto -lm -lelf -ljail -larchive -lz -lbz2 -llzma
Comment 8 Stefan Ehmann 2019-02-10 22:00:56 UTC
Created attachment 201909 [details]
patch for ports-mgmt/pkg to fix pkg-static linking

Attached a patch for ports-mgmt/pkg that fixes pkg-static build on i386 for me (with threads support enabled).

I don't think that patching Makefile.in is the cleanest way to fix this. But it's already patched so it doesn't make things much worse.

Maybe it can be fixed directly in lib/libarchive but my efforts failed.
Comment 9 dgilbert 2021-07-08 19:11:46 UTC
I'd like to see this committed.
Comment 10 Felix Johnson freebsd_triage 2021-09-28 05:22:23 UTC
Created attachment 228222 [details]
bsdtar(1) patch

Document xz:threads option.
Comment 11 Jung-uk Kim freebsd_committer freebsd_triage 2021-09-30 06:59:17 UTC
Created attachment 228274 [details]
Re-enable multi-threading support for CURRENT (335c4f8edb3a)

I believe it is safe to attempt this patch again.  At least, it didn't break pkg on i386 for me.

FYI, I also created patches for pkg to add multi-threading option.

https://github.com/juikim/pkg/commit/67c9465ace456769bb2a41b72ade07c5c46953d6.patch

This patch applies cleanly on ports/pkg (1.17.2) and ports/pkg-devel (1.17.99.5).
Comment 12 Jung-uk Kim freebsd_committer freebsd_triage 2021-09-30 07:38:11 UTC
(In reply to Jung-uk Kim from comment #11)
Sorry, there was a formatting bug in the pkg_create(3).  Try this patch instead:

https://github.com/juikim/pkg/commit/4e57147a0516b287b7c7898ac5a5615e36b536c8.patch
Comment 13 Martin Matuska freebsd_committer freebsd_triage 2022-02-05 08:53:57 UTC
For better maintainability, it would be great to submit any code changes to contrib/libarchive as a PR to upstream.
Comment 14 Jung-uk Kim freebsd_committer freebsd_triage 2022-02-06 18:28:32 UTC
(In reply to Martin Matuska from comment #13)
I agree but the only upstreamable libarchive patch is bsdtar(1) in comment #10.
Comment 15 Martin Matuska freebsd_committer freebsd_triage 2022-02-06 19:02:22 UTC
What about config_freebsd.h? That's bundled upstream as well.
Comment 16 Jung-uk Kim freebsd_committer freebsd_triage 2022-02-07 18:53:08 UTC
(In reply to Martin Matuska from comment #15)
Is it really a static file?  That's ugly. :-(
Anyway, I'll submit a pull request, then.
Comment 17 Jung-uk Kim freebsd_committer freebsd_triage 2022-02-07 21:30:54 UTC
(In reply to Jung-uk Kim from comment #16)
Please see pull request #1664.

https://github.com/libarchive/libarchive/pull/1664
Comment 18 Jung-uk Kim freebsd_committer freebsd_triage 2022-02-07 21:34:48 UTC
(In reply to Felix Johnson from comment #10)
Can you please submit your patch to upstream?

https://github.com/libarchive/libarchive

Thanks!
Comment 19 Jung-uk Kim freebsd_committer freebsd_triage 2022-02-07 21:39:01 UTC
Created attachment 231620 [details]
Re-enable multi-threading support for CURRENT (3ec0dc367bff)
Comment 20 Jung-uk Kim freebsd_committer freebsd_triage 2022-02-09 03:10:17 UTC
(In reply to Jung-uk Kim from comment #18)
I created a pull request myself.

https://github.com/libarchive/libarchive/pull/1665
Comment 21 Jung-uk Kim freebsd_committer freebsd_triage 2022-04-08 13:18:36 UTC
Created attachment 233058 [details]
Re-enable multi-threading support for CURRENT after 7f815d4f128f

Rebased for 7f815d4f128f
Comment 22 Mark Linimon freebsd_committer freebsd_triage 2024-01-02 20:07:58 UTC
(In reply to Jung-uk Kim from comment #20)
^Triage: note that the pull request was accepted some time ago.
Comment 23 Mark Linimon freebsd_committer freebsd_triage 2024-01-02 20:09:11 UTC
^Triage: this PR is now OBE since the upstream pull request was granted.