Possible issue with processing pages (promises)

[markomanninen]

I found this problem on my other plugin using a similar pre-processing structure that is used on image-captions. While Q.all prevents the page hook to be called before the init hook is done, processPage calls in the init hook are still async. This means that even navigation items (files) are iterated in order, page processing will be async. This may cause image lists to become out of order. Using additional promise code (reduce thing), this possible problem can be solved.

https://github.com/markomanninen/gitbook-plugin-image-captions/commit/03c98dcfc4e4a872da474e072db5a482c4f82d90

The problem won't be visible on short test pages with few images, I guess. But on a real book project, with hundred pages and tons of pictures, it will become apparent.

You may want to recheck this scenario, Tomas.

[todvora]

Hi Marko, Thank you for the warning and your solution. I'll try to write some tests for this scenario and integrate your fix. First thought - if we serialize all the calls, will be the processing noticeably slower? Any experience?

Best regards, Tomas

[markomanninen]

I have not seen any speed decrease, by eye. Thought I have not profiled programmically either. Is there any easy to use build profiler / timer for GitBook project? On the other hand the output is static anyway, so speed is not that critical in my opinion.

[todvora]

I merged your change and all the tests are passing. On my current test cases isn't any slowdown observable. Thank you for this contribution Marko!

[todvora]

Regarding build profiler - you can try the gitbook-tester in the same manner as in this plugin. Every test case outputs total time of execution. So it should be possible to see a potential slowdown, if you generate big enough book and let it build by the tester.

[todvora]

Hi Marko, The latest version of your code worked ok for me and passed all tests. Unfortunately there has been a error introduced (#7).

I've created new test case, that validates exactly your observation with mixed up order of image global indexes. The problem could be observed immediately.

Then I changed the logic around Q.all little bit. First it parses all the images in parallel. Then they are sorted according to the navigation object. During the refactoring, some other things has been slightly changed. Mainly scoping (this/that), global images variable and so one.

It would be cool, if you had some time and could check those changes and source code. There are lots of features, you designed and you know best, if they work as expected.

Thank you very much!

Best regards, Tomas

[markomanninen]

Hi Tomas,

brilliant work you have done! I took a look (actually few times to the code). Refactoring looks nice, it clearer to read now. Actual fix to the map/reduce dilemma is still strange. Did you find the precise point when reduce index error was found? What caused it? I'm glad you could verify the number order issue on big lists. I had too limited time to test with given open source books, but it looks reasonable that image list is sorted at the end according to menu order.

Have a great time with projects! -Marko

[todvora]

Hi Marko, Thank you! I haven't been able to find the exact reason, why the map/reduce failed. Nevertheless the bug could be verified on books from #7. I wanted to limit number of side effects (global vars) from our processing pipeline too, so I rather invested time into refactoring.

With Gitbook v3.0 are the internals and API totally different, so it will be needed some kind of abstraction and refactoring again, to maintain compatibility with 2. and 3. releases. Stay tuned :-)

Best regards, Tomas

[todvora]

Refactoring done, lots of changes included. Closing this issue now, but feel free to reopen it or create a new one, if there is something more to solve.