Support concurrency #83
Conversation
Stress-testing shows `find-headings` method triggered numerous errors when run in a multi-threaded env, so convert it into a method (this introduced additional API point, but as rewriting it into `given/where` would be far less efficient and private multi are not supported, this is the way we go here) and make the whole class a monitor that fixes some bugs. Additionally, add a simple workaround for internal bugs triggered by Template::Mustache rendering in concurrent environment. This deals with the most annoying cases of concurrent (ab)use of Pod::To::HTML module, surviving rendering whole documentation for 100 times with 24 parallel workers.
|
I am thinking about reverting this PR, unless you give me a very good reason to do so. We don't know if it really addresses what it says it does, since it does not add new tests to the test set. And, on the other hand, it's built on top of another PR that is apparently causing some issues. |
A (IMO) good reason not to revert it just now: reverting needs a time and a place. In general, it is not a solution. Some issues were solved by this PR but it introduced regressions. A real solution: increase test coverage and fix regressions. A temporary solution: revert. AFAIK, we do not update docs automatically. Website is not broken right now. Website was not updated very often anyway. Is it impossible to wait until tomorrow for me to deal with regressions? We update website manually when possible, but it is absolutely very desperate to do it right now? We are not pressured to do a revert and can provide a real solution instead of messing with history. I don't get it. |
|
There was no good reason to merge it anyway, since it was not proved (by a test) it solved what it was supposed to solve. |
Tests do not prove there are no bugs at all. Especially it is very true for concurrency issues and this PR deals with a couple of those. But such heavy stresstesting is not suitable for most users. Not everyone would want to render docs for 100 or for 1000 times using 24 or more threads. Possibly we can go with |
|
I'm not rejecting it, it's already approved, and I did approve and merge it. |
So I did a bit of investigation for #63...
It seems
find-headingsis unsafe, so made it into a method and protected with monitor. ??? Problem is gone.It seems
Template::Mustachetriggers some rakudo/moar(?) issue as well, as it refused to compile some pages (usually 5 plus-minus a couple from the whole documentation, always different ones) with a bogus error (self being null). So created a special loop catching issues doing some retries. It fixed the problem I saw. By the way,Cro::WebApptested had no issues like this one and was on average 2.5x to 5x times faster, but to use it we need a smooth API at hand.Did some crunching with 24 threads rendering whole documentation 100 times in a row. In three crunch sessions saw a moarvm bug once, which is, I suspect, known and so very obscure we probably don't want to deal with it here (I saw it reported in different places).
Otherwise, it became a lot more stable unless you really stresstest it, so I believe the ticket can be closed.