Skip to content
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.

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

Parallel processing is broken, even with --parallel=1 #1938

Closed
coke opened this issue Apr 17, 2018 · 13 comments
Closed

Parallel processing is broken, even with --parallel=1 #1938

coke opened this issue Apr 17, 2018 · 13 comments
Assignees

Comments

@coke
Copy link
Collaborator

coke commented Apr 17, 2018

Because we are using ".allof" to wait for Promises, we are losing the fact that some of the pod processing is erroring out. - allof just makes sure the promise ended, not that it worked.

So far I've found issues with files that have:

=head 3 C<thing>

and nested tags like:

X<C<a thing>>

Basically, there are several places in the code where we are assuming something is a Str, when in fact it's a Pod object.

@coke coke added the build label Apr 17, 2018
@coke coke self-assigned this Apr 17, 2018
@JJ
Copy link
Contributor

JJ commented Apr 17, 2018

But are the two things related?
I seem to remember that worked correctly... at least until some time ago.
And the problem is anyway that we only have htmlify.p6 as a reference of how htmlify.p6 should behave. So a test suite is essential...

@JJ
Copy link
Contributor

JJ commented Apr 21, 2018

If #1947 depends on the the --parallel flag, maybe we should eliminate that completely. Is it really making things faster?

@dogbert17
Copy link

dogbert17 commented Apr 21, 2018 via email

@coke
Copy link
Collaborator Author

coke commented Apr 23, 2018

If it's broken, it doesn't matter that it's faster.

I've ripped it out in the coke/build branch, may add parallel processing back in later; the main issue here is the "Promise.allof", which hides failures encountered when processing some pages; if you just switch it, the build will fail on the first page. I have more changes in coke/build to work around the issues, but they are just workarounds.

@tisonkun
Copy link
Member

make html takes over 30 minutes on my local environment and longer with --parallel=n. I suspect highly wrong parallel does harm to efficiency instead of speeds up.

@JJ
Copy link
Contributor

JJ commented Apr 23, 2018 via email

JJ added a commit that referenced this issue Apr 24, 2018
Since it's not actually improving speed, and it causes hangs in some
operating systems such as OpenBSD. This would close #1947, and refers
to #1823 and also #1938, only I don't close that one since I guess the
point of it is to fix it.
@coke
Copy link
Collaborator Author

coke commented Apr 30, 2018

d3db402 by @zoffixznet may have resolved this issue.

@JJ
Copy link
Contributor

JJ commented Apr 30, 2018

We'll have to check and close the issue if needed.

@JJ
Copy link
Contributor

JJ commented May 1, 2018

It dies when you do --parallel=2.

 11/53: doc/Language/control.pod6                => language/control
An operation first awaited:
  in sub process-pod-dir at htmlify.p6 line 269
  in sub MAIN at htmlify.p6 line 199
  in block <unit> at htmlify.p6 line 1003

Died with the exception:
    No such method 'level' for invocant of type 'Pod::Block::Para'
      in block  at htmlify.p6 line 627
      in sub find-definitions at htmlify.p6 line 503
      in sub process-pod-source at htmlify.p6 line 318
      in block  at htmlify.p6 line 265


@JJ
Copy link
Contributor

JJ commented May 1, 2018

--parallel=1 and no parallel take pretty much the same in my system. ~ 190 seconds. No wonder, since it's the default value...
So maybe we should do #1823 and fix it and/or eliminate it.

@coke
Copy link
Collaborator Author

coke commented May 1, 2018

parallel=1 on the command line is exactly the same as passing no arg. You're running exactly the same code.

the fact that parallel with numbers > 1 is broken has been discussed elsewhere. This ticket was specifically to point out that the theoretically non-parallel case of using 1 (either explicitly or implicitly) is (was?) also broken.

@JJ
Copy link
Contributor

JJ commented May 1, 2018

I didn't know that. Thought was kind of a flag. Anyway, let's close it if it's no longer broken.

@JJ
Copy link
Contributor

JJ commented May 7, 2018

And closing...

@JJ JJ closed this as completed May 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants