Win32 parallel testing #52
Conversation
With 1 million tests processed, NYTProf shows this sub was called once for each test processed, but the print block only executed 1600 times. So dont compute the output method name, and dont fetch the current test number, if we aren't going to use it.
… behavior between runs" This partially reverts commit 4d4d3f0. In "Unknown arguments to TAP::Harness::new" croak, sorting was kept, but moved into the branch where it is needed.
new() went from 17.2 seconds (/1000000) exclusive time nytprof to 12.2 seconds after this commit.
Also combine subs with identical bodies. On Win32, memory usage sampled with "perl -MTAP::Parser::Iterator -E"system 'pause'" caused memory usage to drop from 2,688KB to 2,672KB after this commit for me. next() dropped from 68us inclusive to 44us with nytprof, for comparison next_raw is 39us. next() is called once per test so it is hot.
If something is very wrong with the TAP stream, don't silence $@ die errors.
TAP::Formatter::Console::_output doesn't exist, TAP::Formatter::Console inherits _output from TAP::Formatter::Base::_output so patch that. In some cases (future Win32 Parallel testing implementation) the TAP::Formatter in proverun.t is a TAP::Formatter::File, not a TAP::Formatter::Console as before.
Increase likelyhood of race conditions or bugs being revealed. Such bugs being IO APIs start mixing up and loosing track of which IO block came in on which fd/stream/fileglob/proc. Increasing number of procs is adding more opportunity for OS scheduler randomness. This patch helped diagnose and make almost always reproducable a race condition bug I created and fixed.
This commit breaks on unix. Next commit fixes unix.
Unix.pm is a modified (most changes are deleting things) pre-Win32-Async Process.pm
t/nofork-mux.t failed with XSConfig installed. Mock::Config was rejected as Test-Harness can't have any non-core dependencies.
Iterator::Process::Windows objects were all being dtored at global destruction (a mem leak), not in TAP::Parser::next() where Iterator::Process::Unix objs were dtored.
Previously TAP::Parser::Iterator class was pull based, now add an optional
push design so a central event loop can add data blocks to a Iterator.
The existing select() design is O(n), the Win32 Async layer is a FIFO/LIFO
O(1) queue (a MS IOCP). Adding a would_block() or can_read()
(AKA "return scalar(@{$self->{'buf'}});" } method to the
Iterator::Process::Windows() is a bad design as that is O(n) so do it
the right way by pushing data into the Iterator.
Remove unused $state var from day 1 of sub pragma in commit "Pragma support." Combine 2 getter sub calls into 1 property in has_problems. Push the value into the obj instead of fetch it. Previously $self->tests_run() executed 4 times for every .t proc according to single stepping and NYTProf. Call it only once per .t proc for efficiency. tests_run is an integer.
save memory by not loading Carp into a typical EUMM test_harness proc although this commit doesn't actually stop loading Carp.pm due to p5p controlled modules that will have to be fixed one day TAP::Harness remove require Carp/use Carp because Carp is never directly called in the .pm Scheduler::Job and Scheduler::Spinner dont use Carp, remove it remove ops+pad var in Aggregator, less memory TAP::Parser::Aggregator and TAP::Parser::IteratorFactory switch to a lazy loaded _croak/_confess from TAP::Object, no need to execute "require Carp" when it wont even croak most of the time TAP::Object remove unused my vars
|
Bump, this PR will soon be able to drive. |
|
Thanks for the bump! You can see this PR has conflicts that GH is saying needs resolving? Also, Travis is gone now, and there's now GitHub Actions that does Windows (though Perl <=5.20 is failing, that wants fixing). |
This PR is rather big and complicated. Realistically it's a huge hack that was needed because Test::Harness's guts are broken. But IO is also kind of broken on unix, so if anything this branch is an argument for a much deeper rewrite of Test::Harness. |
This PR is 16 commits from 2016, Most of the 16, are irrelevant to adding Win32 parallel tap feature. I can remove them easily. The 16 commits are intentionally very tiny patches for git history reasons. And basically can be split into 16 PRs if someone wants me to really make 15 new PRs in row for This is a very long post b/c im including API design choices in this post for archive reasons if anyone looks 10 years from now at this.
I will just say right now bluntly, P5P will never allow https://metacpan.org/pod/EV http://software.schmorp.de/pkg/libev.html or https://libuv.org/ into core as a dependency. CPAN Note, for the rest of this post, what I am calling This is a private module exclusively used by and part of Test::Harness. External usage of this module on CPAN is not supported. LINK__TO__TEST__HARNESS__MCPAN APipe.pm 2016 is 813 bytes, effectively empty. APipe.xs 2016 is 30,076 bytes. 30KB/800 lines of XS. 800 lines or 400 lines for v0.01 of XS owned by Perl Community, is very different from 3MB of FOSS code owned by Google LLC. The 2016 API half-Contract between APipe.xs 2016, has way too much This PR looks huge, but look at the 16 commits individually. The combined patch is huge because it has alot of irrelevant to this ticket generic perl optimizations thrown around from 2016. I can easily remove them but I want some input, if anyone sees something that could be changed, on this 2016 branch before I try shaving new LOCs, into a much smaller 2024 branch. Counting that, the 2016 API, is 9 tokens total. If this a JAPH contest and not CPAN code, the API can be reduced to 3 or 4 tokens. So because the 2016 API, which runs correctly as a POC, is 9 tokens. Any realistic final API between Harness and APipe is 5 tokens min, 18 tokens max. JAPH contest style is monkey patching 3 PP keywords. Writing all the code for Harness.pm to use https://metacpan.org/pod/UV is an extreme forklift, or might be abandoned half way through for some unsolvable API design reason by the author. "Author" is anybody and any perl dev, not necessarily not me. My personal nickname for the code in 2016 and right now 2024 is a "TAP::Parser::Iterator::Process::Windows::XS" object. I'm using the name So because my plan is that APipe.pm stays empty forever. The "PP" half of APipe::, whatever the PP half is, big or small, has to stay in Harness.pm repo/tarball, so any grep of Harness.pm code base in future, the PP half will be seen by a future maint person with a grep. Anyone who wants to touch the portable POSIX half of Harness.pm has to see the Win32 PP side, for ideas etc. Its not a Perl thing to split all condition blocks for BSD, OSX, VMS, Solaris, etc, into separate tarballs, because those aren't real "OSes". Or the project owner has a policy BSD and Linux are commercial products that can't be mention in our free POSIX software. You see that file called "LICENSE" in Linux? Its not free software. APipe.xs might never get CPAN release 0.02. It only uses kernel32.dll, and MS WinAPI guarantees kernel32's ABI vs .exe'es/.dll's, for eternity for each MS CPU arch. Kernel32's API/WinNT CreateFile/WinNT IOCP functions, are frozen since NT 3.51/1995. They probably are frozen since OS/2 1.0's SDK 1987. In 2016, the most controversial choice I did is, no Windows 98/ME support. Win32 Perl and Harness.pm on Win98 Kernel will just call the universal current PP Less controversial is APipe.xs calls There are easy fallback shims floating around on google on emulating Once you get access to There is no way ever, to emulate IOCP APIs on Win 95/98/ME. Nobody has ever tried, and you won't get anywhere. FILE_FLAG_OVERLAPPED (win32 async) is documented by MS as "ignored-Win 95/98/ME". The API contract from The Win16 kernel from Win 3.1, is the same code as the Win95/ME kernel. Nothing function call, or flag in a struct, exists on the driver side of that Win16 kernel, 3.1 or ME, that allows "return to ring 3 usermode half way through read(), and I refuse to fill in So summary, 95/ME Perl users filing tickets against Harness.pm. Rejected-legacy/portable block code in Harness.pm works perfectly. NT 3.51 Perl users filing tickets against Harness.pm, Fix it-probably 1 hour of work. Its a copy paste from google of backporting So I can't really finish APipe.xs or move from alpha/a rough hack stage, unless APipe.pm/.xs has no provisions by me, for any code reuse by "CPAN" or anyone but Harness.pm. The 2 should be in the same repo, but aren't in the same repo b/c I might move some 15-30 statements from 2016 Harness.pm to 2016 APipe.pm/.xs or other way around for a cleaner API boundary or something, but its minor. Note, the new file called in a refactor, sort of could get renamed, or certain PP subs, moved to the 2016 almost empty APipe.pm file for code style reasons. IIRC in 2016 leont at the hackathon had no comment on API split, since getting the Win32 Parallel POC test to compile and execute atleast once ever, was a higher priority than discussing method names. I'm not too sure about new file The real change is basically and splitting Note, all API or hooks for TAP Win32 Parallel, can ONLY go in the current https://github.com/Perl-Toolchain-Gang/Test-Harness/blob/master/lib/TAP/Parser/Iterator/Array.pm and https://github.com/Perl-Toolchain-Gang/Test-Harness/blob/master/lib/TAP/Parser/Iterator/Stream.pm are NEVER USED by blead perl, and NEVER USED by EU::MM. Both of those classes, also don't implement The only real changes from a Harness.pm view are 2 things. A sub (Win32 XSUB) must be called to read a child proc's STDOUT object to get a scalar string of TAP. Can't do 2nd change of not calling PP keywords Any and every call 2016 Harness.pm does, to fake
This keeps very little changes, code wise, in the current unix/posix/win32/cross platform portable half of Harness.pm code. 2016 POC Harness.pm never calls PP select() on WinOS, The result is, "what does PP keyword select() do on Perl on WinOS?". Best answer, is no answer. That question never needs to be answered in Harness.pm. Nothing changed. Again, biggest change TAP::Harness must know on Win32, a child proc's STDOUT object can't be a a real PerlIO/perl glob object but is a generic blessed perl object, so no calling The rest of the parallel TAP parsing implementation is just Win32 specific Async IO stuff that lives in XS in It can be done as a contest (JAPH style), but its too much code to fake, to implement Win32 parallel harness feature, with zero code changes to Harness.pm. Also to fake it, there will be alot of pointless runtime overhead, and one time writing lines of new code, and much more time/difficulty/super brittle for any future maintenance or debugging. To do these 3 things by aggressive monkey patching Monkey patching is almost never production grade or CPAN grade code, and almost never the right way to solve anything. So this has to be solved at a Harness.pm level with an small new package/class/API in Harness.pm to "other side" .pm, not monkey patches at 3rd party level. And P5P won't accept any code into its repo that looks like a JAPH. Since whatever is the final code, it had to be P5 core include quality, that is the ultimate goal. Basically Harness.pm has to call method XSUBs 1 for 1 replacement, on Win32, instead of PP keywords on child proc "STDOUT" "objects". The Win32 code is pretty basic XS code doing Win32 specific Async kernel IO, along with a serialization/msg queues of var length string buffers, coming off child procs. basically what Harness.pm already does, but calling Win32 specific methods instead of certain PP keywords when doing IO. The real backend (APipe.pm) does 2 tricks internally to pull off parallel TAP. Fake The 1st fake I don't want to really change or touch or implement Harness.pm/Multiplexer's current Also In theory Harness.pm, with BOTH a TCPIP socket and pipe FH/process, is supposed to work on Unix and Win32???? right?? So I assume the correct design is Harness.pm must do PP select()/->win32specialfakeselect() one after another in the select loop on WinOS or atleast leave provisions. Does anyone ever actually use Harness with TCP sockets or its dead code? semi jk I dont want Describing fake Im keeping in mind to minimize pages of new code for Harness.pm/APipe.pm maintainers, me, and anyone else in the future. No I know nobody in Toolchain/Perl community, will ever religiously synchronize anything maintainers of P5P core/IPC::Open3/IO::Select do, with near identical subs/funcs/algorithms/lines of code that live in Harness.pm or APipe.pm that are doing "perfectly emulating POSIX stdio on WinOS" attempts. I don't want to ever look back on this PR or this code, me, or any other Win32 Perl user. I assume a large city bus will run me over 1 day after Perl 5.42 or Perl 5.44 tarball is released. So less new LOC in Harness.pm or APipe.pm side is better than for any v0.01. Its best to avoid, tons of new LOC that exist in 1 sub for some other new LOC in another sub (caller/callee), in the same exact commit to Harness.pm, to see "POSIX stdio" API behavior. between callee/caller pairs on Perl for WinOS. This is the end of the design theory of this PR.
I wonder who gets the joke. APipe.xs 2016 does not have a fake Semi-off Topic This is something I saw last week in 5.41 blead with a C debugger, but I stopped research because I was trying to do something else in blead C code. POSIX STDIO, Perl 5 yylexer() and the Perl 5.0-5.41 the C PerlIO P5P maintained layers, have a baited dangerous mousetrap, the name of the mousetrap is '''getch()``` or https://pubs.opengroup.org/onlinepubs/9699919799/functions/getchar.html So, hypothetically if you layer Perl 5 is worse than POSIX, its not a mousetrap like in POSIX but instead a landmine with C4. https://perldoc.perl.org/perliol
So lets layer PerlIO 5.0-5.41 has a built in safety limit in real life, If you trick PerlIO to do the above behavior, %99 or %100 of the time, it is 4096 bytes or in Perl 5.41 I think I saw 8192 bytes getting bounced by PerlIO. 2ms-5ms overhead of CPU anywhere in any Perl production app, will never be noticed by any XS dev, or PP dev. 4 ms is always invisible to Perl+TK GUI+User's mouse and always invisible to Perl+CGI+Nginix. This isn't a P5P ticket yb/c I didnt research enough what I think I was seeing in my C debugger with 5.41 and yylexer(), and my blead perl build had unstable changes/random code hacking in perl541.dll by me, |
|
@bulk88 I'd say pulling out parts you believe are uncontroversial and making separate PRs is definitely a good course of action. Just looking at 2 commits, I don't believe the "add CI testing" should be there at all now; and the VMS one looks like a good idea by itself. If there is a wider async IO problem existing on Windows, that might be fixed elsewhere first? Apologies for my ignorance, but you refer to |
QAH branch submitted for review/shipping.