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

RFC: Compile-time wiring checker #51

Open
kaishh opened this issue Feb 19, 2018 · 3 comments

Comments

Projects
2 participants
@kaishh
Copy link
Member

commented Feb 19, 2018

The only meaningful way to check a plan is to just run the planner inside a macro.

That means the check has to be done at top-level, when all modules have been merged into one module. This also means that module merges have to be defined at type level or macro/compiler-plugin level.

What meaningful checks are there?

  • ensure all dependencies are wired (there are no imports)
  • when checking a single module, ensure that exactly the provided imports are there
  • statically check configs: Ensure that all required keys are defined in application-reference.conf and that there are no redundant keys.

Type checking dynamic modules:

Let's say there is a module that we change at runtime, depending on config, that means we have 2 possible static configurations: mod1 ++ mods and mod2 ++ mods. If we naively check both configurations, we'd need to run the macro twice. If we add more modules, the number of possible configurations grows exponentially. At 16 dynamic modules, we'd have to run the macro 65536 times, which would definitely slow down our compilation!

We may do it another way though:

define top-level module as a set of Variants of modules (TargetPoints in old izumitk)

// Modules with `||` have dynamic variants, modules without are fully static
new ProgramDef {
  val moduleVariants = Seq(
    (customerRepoImpl1 || customerRepoImpl2)
    ,(userRepoImpl1 || userRepoImpl2)
    ,(facebookNotifications || twitterNotifications)
    , staticModules // no `||` operator here, this part is static and doesn't change
  )  
}

Now extract the full static part of the program:

  • Extract all the produced bindings in static modules
  • Extract the intersection of produced bindings in dynamic modules (take produced bindings that are common in all the variants) // You can also say that we are extracting interfaces from modules. We can do that too. Instead of calculating the intersection between all variants = just require them to define and inherit an interface and check the interface. We'd have to use explicit interfaces to support plugins

Now check the dynamic part of the program:

  • For each variant in a dynamic module, take the dependencies and check that every dependency is always fulfilled by the full static part of the program or by the module itself.

That means that instead of running the macro 65536 times, we only need to run it once, and check each variant once.

This typing mechanism is somewhat more restrictive, but it rules out invalid cases such as for example, there are 2 target points: Impl, Logger and 4 modules DummyLogger, DummyImpl, RealLogger, RealImpl.

DummyImpl can only work with DummyLogger and RealLogger can only work with RealLogger:

class RealImpl(logger: RealLogger) extends Impl

So there are 2 valid configurations possible:

Logger = DummyLogger
Impl = DummyImpl

and

Logger = RealLogger
Impl = RealImpl

But there are also 2 invalid configurations:

Logger = DummyLogger
Impl = RealImpl

and

Logger = RealLogger
Impl = DummyImpl

In our typing scheme the user will get errors such as:

In variant RealImpl of Impl: RealLogger is not part of static context. Provided dynamically by RealLogger variant of Logger. Logger has variant DummyLogger, that does not proviode RealLogger.
In variant DummyImpl of Impl: DummyLogger is not part of static context. Provided dynamically by DummyLogger of Logger.  Logger has variant RealLogger, that does not proviode DummyLogger.

To remove the error, user will have to remove the Logger target point and put the logger inside the *Impl modules:

object RealImpl extends ModuleDef {
  make[RealLogger]
  make[Impl].from[RealImpl]
}

Now there are only 2 possible configurations, and both are valid:

Impl = RealImpl | DummyImpl

IMHO this is a good thing, even though the user can't split the Logger inside into its own target point, the split was nonsensical to begin with and only added invalid configurations to the program.

Depends on #102

NB type level impl: HList Cons on single bindings could slow down the compiler a lot. should instead append HLists of HLists

NB performance: Running the planner at every recompile may not be fast at all! the user would have to be provided with performance diagnostics and a way to disable the checks or enable them selectively (i.e. disable for dev, enable in ci)

@kaishh kaishh added the distage (di) label Feb 19, 2018

@kaishh kaishh added this to the distage-1.0 milestone Feb 19, 2018

@kaishh kaishh self-assigned this Mar 15, 2018

@kaishh kaishh changed the title Compile-time plan checker for static plans RFC: Compile-time plan checker for static plans Jul 4, 2018

@kaishh kaishh changed the title RFC: Compile-time plan checker for static plans RFC: Compile-time wiring checker Jul 4, 2018

@kaishh kaishh added the enhancement label Jul 14, 2018

@pshirshov

This comment has been minimized.

Copy link
Member

commented Jul 25, 2018

Once we have roles mechanism in place and refactored it shouldn't be a problem to to the static check part for PluginLoaderPredefImpl.

The ideas about grouping logic (including #292) should be proven by runtime experiments first then adopted for static check.

@pshirshov pshirshov modified the milestones: distage-1.0, 1.0 Jul 25, 2018

@kaishh

This comment has been minimized.

Copy link
Member Author

commented Jul 26, 2018

@pshirshov suggested a very cheap way to do this vs. #102 :

If we have a set of Plugin classes in separate artefacts
And a main launcher in separate main artefact

We can run a macro in main and instead of taking types of all of our bindings, we can take JUST the types of our PLUGINS - since plugins have to have a null-arg default constructor, we can INSTANTIATE them directly within a macro (since they are in separate artefacts, we already have their symbols) and run the usual planner logic.

@kaishh

This comment has been minimized.

Copy link
Member Author

commented Oct 7, 2018

The cheap way outlined above was implemented in #411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.