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

Use fibers to minimize redundant work #985

Merged
merged 4 commits into from Nov 17, 2016

Conversation

Projects
None yet
1 participant
@ddfreyne
Member

ddfreyne commented Nov 16, 2016

Summary

Perform compilation in a resumable fiber.

Motivation

The compilation of an item rep can currently fail due to an unmet dependency (a Nanoc::Int::Errors::UnmetDependency exception will be raised). This is suboptimal for two reasons:

  • All work done on compiling this item representation is lost, and will have to be started over.

  • In a worst-case scenario, an item representation that has many hard dependencies on other other item representations might be compiled in a way that causes the item representation to be compiled over and over again, and failing due to unmet dependencies over and over again. This could drive the compilation time up by orders of magnitude.

The approach in this PR eliminates used work. When an unmet dependency occurs, compilation will continue at the point where the unmet dependency happens.

Detailed design

The main change is in lib/nanoc/base/compilation/compiler.rb, whose diff is best viewed with ?w=1. The fiber itself does the work, and the fiber runloop does the notifications. Fibers are retained between #compile_rep calls, in order to make them resumable.

The raise in raise Nanoc::Int::Errors::UnmetDependency has been replaced with Fiber.yield. This is the mechanism that allows compilation to be suspended rather than aborted. This also makes the icky ItemRep#forget_progress unnecessary, and has been removed.

The compile command has gained a few tests for the file action printer and the timing recorded. These have never been tested before, so it’s good to finally have useful tests. I’ve also introduced Timecop, which makes time-based tests more reliable and faster to run (because there is no need for #sleep). The compile command also had to be modified to take the :compilation_suspended notification into account.

The #compile_rep method now has unit tests, which unfortunately are somewhat ugly as the method is private, and has a bunch of dependencies which need to be injected. This might mean that this code could/should be extracted; perhaps a SingleRepCompiler might be useful. Such a refactoring is out of scope, though.

To do

  • Save fiber and resume it
  • Retry after yielding fiber
  • Record timing properly
  • Fix compiler stack

Tests:

  • #compile_rep stack
  • #compile_rep dependency tracking
  • #compile_rep notification emission
  • compile command timing for file actions
  • compile command timing for filter aggregates

Related issues

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Nov 17, 2016

Member

This change is not compatible with the presence of the compilation stack. It might be worth removing this stack.

Member

ddfreyne commented Nov 17, 2016

This change is not compatible with the presence of the compilation stack. It might be worth removing this stack.

@ddfreyne ddfreyne added this to the 4.3.8 milestone Nov 17, 2016

@ddfreyne ddfreyne merged commit 63fe7e7 into master Nov 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ddfreyne ddfreyne deleted the fibers branch Nov 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment