Closed #226.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#event-2345662694___
Rpm-maint mailing list
As #695 supersedes this I am closing this PRs.
Thanks for your work and pushing this issue forwards. Those PRs clearly
wouldn't be there if it wasn't for you!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Yes, as already mentioned in an earlier comment to this PR:
https://github.com/rpm-software-management/rpm/pull/226#issuecomment-425422760
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@pmatilai I trust that the last two commits in my set (make string pool
operations thread-safe; avoid use of static data) are already merged into
master in some form? Don't see those fixes in #695.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
For interested parties, I submitted a more elaborate version as #695 with
further code cleanups and additional parallelization of file classifier.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
FWIW, thread protection added to the string pool as of commit
7ffc4d17ffa8f87bd9107b5dc4d9e25daadd14ae so that part of these patches can be
dropped. Brief testing showed no problems with the rest applied, but I don't
have any real testcase for parallelism at hand.
--
You are receiving this
Is it possible to get this accepted? I've been testing it on openSUSE's build
services, with some very rough (semi controlled) benchmarking the speedup's
aren't huge a medium size package that splits out into multiple packages might
save 10 seconds off a 5-6 minute build but when you take that
Never mind it seems that the patch is not being applied correctly here
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@ignatenkobrain testing your patches in the Yocto project (with some extra
debugging statements) on openSUSE's open build service instance its clear that
the packages are still being built sequentially I'm not sure why yet, but
here's some debug output below.
Some of us at SUSE are quite keen
Ok, I pushed the first and the last patch as a sign of good will.
The problem here is that librpm (which rpmstrPool is part of) but also
librpmbuild are libraries that might be used in applications running other
threading implementations.
Also rpm already uses pthread - although not very
Shouldn't the time have come to review this again for finally merging this one
after more than a half year now?
With rpm 4.14 out, it should be the right time to merge this one now, no? :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view
@ignatenkobrain: tex (with many packages all with few files) isn't the best
benchmark to illustrate a parallel packaging speedup. There are a finite number
of threads in the thread pool (which you seem not to have reported or
controlled for, sigh).
Try a benchmark with a few packages with lots
@kanavin, even 8 minutes speedup is good ;)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#issuecomment-325989203___
@ignatenkobrain The reason we see a lot of speed up in Yocto with this patchset
is that building the sources happens outside of rpm, and we only use rpm to do
packaging of binaries. If the bulk of the above time is spend building texlive
from source then of course the final difference won't be
It would be nice to run dependency generators in parallel as well (although it
might disallow us in future to do some advanced generators which would take
subpackages into account, but AFAIK no one is planning to work on something
like this in foreseeable future so doesn't seem to be a
> Finished binary package job, result 0, filename
> /home/brain/rpmbuild/RPMS/x86_64/hello-debugsource-1-1.fc28.x86_64.rpm
not sure if we should print those messages...
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
P.S. it has thousands of subpackages
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#issuecomment-325729112___
Rpm-maint
I will try to build texlive with and without PR and will report results :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@ignatenkobrain Sorry, forgot to push the fix. Now fixed.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@kanavin
```c
spec.c: In function 'getBuildTime':
spec.c:214:9: error: 'errno' undeclared (first use in this function); did you
mean 'h_errno'?
errno = 0;
^
h_errno
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email
Reopened #226.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#event-1225606832___
Rpm-maint mailing list
Closed #226.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#event-1225606736___
Rpm-maint mailing list
let me reopen PR to run CI tests
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#issuecomment-325657326___
Rpm-maint mailing
@Conan-Kudo @ignatenkobrain I have rebased this against master now. I checked
that it builds, but did not run it against any spec files.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@kanavin Now that rpm 4.14 has been branched, could you rebase them for review
to merge into master?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@kanavin any news here?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/226#issuecomment-318925604___
Rpm-maint mailing list
BTW it'd probably be easier and less intrusive to make buildtime and buildhost
part of the spec struct, initialized in newSpec() or so. AFAICS all the
relevant places are receiving spec as the argument already, with the exception
of writeRPM().
--
You are receiving this because you are
This seems to work correctly on my Mac OS X 10.10 system, though admittedly I
don't have a great test case with lots of subpackages. With ~3 subpackages
(progs, libs, devel), it seems to work fine.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
I've no prior knowledge of OMP but doesn't look half bad on first sight.
Cosmetics aside, we can't really have half the codebase doing half-assed manual
pthread locking here and there and another half using OMP, AIUI this is
undefined behavior and the string pool is heavily used by non-build
I have now updated the patchset again, addressing the crashes with old gcc, and
Jeff's comment above. And this time they should be rebased correctly :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
You need to make sure that RPMTAG_BUILD{TIME,HOST} in both the *.src.rpm and
the binary packages is identical. This was done originally with static
variables, but can also be done by setting these variables in one place once,
and then using the stored values where necessary.
--
You are
There you go, I force pushed the reworked patches. It is 100% pure openmp now :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
kanavin commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
Alright, I'll rework the patches with openmp, and re-do the pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to
proyvind commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
openmp, unless for some reason would turn out to be unfeasiable, would be the
definitive candidate IMO given it's standardization and implementation
ignatenkobrain commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
openmp?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Conan-Kudo commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
It'd be pretty hard for me to compile this on Mac OS now, as the Netscape stuff
is difficult to bootstrap.
--
You are receiving this because you are
ignatenkobrain commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
@kanavin NSS is not hard dependency, it has support for openssl and other
crypto backend and I would say that NSS is hardest to bootstrap so we
kanavin commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
rpm already has a hard dependency on nss, and nss has a hard dependency on
nspr, so it seemed obvious to utilize the thread pool API from nspr. Why you
are
ignatenkobrain commented on this pull request.
> @@ -14,6 +14,8 @@
#include
#include
+#include/* NSPR thread pools */
I'm kinda reluctant for such external dependencies
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly
cool!
I'll try get around reviewing it myself at least when I find the time, then
provide feedback and/or my approval of (FWIW). :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
40 matches
Mail list logo
