Glow: Pass Context at runtime not compile time

Created on 22 Oct 2018  路  4Comments  路  Source: pytorch/glow

In order to facilitate multi-threaded execution context passing needs to be moved from compile time to runtime. This will break the current API!

At compile time, constant weight allocation will remain unchanged, however Placeholder memory will no longer be allocated at runtime. The expected sizes will be calculated from the the Placeholder types.

At runtime (ExecutionEngine::run) the context will be passed in and Placeholder memory will be assigned to point to the allocations in the context.

When done context will no longer be a parameter of the compile function. ExecutionEngine::run and function::execute will expect a context parameter.

This change will be done in stages to minimize disruption.

  1. ExecutionEngine::run(Context &ctx) will be introduced. This is the new execution flow. When the bugs are worked out it will replace run. Tests will be ported to use run(ctx).

  2. CompiledFunction::execute(Context &ctx) will be introduced, similar to run(ctx) this will replace execute() when the work is finished. Tests will be ported to execute(Context &ctx).

  3. execute(Context &ctx) will be updated to handle memory allocation at runtime.

  4. Once runctx and execute(ctx) are working and all tests pass run(ctx) will replace run and execute(ctx) will replace execute. At this point any external code will need to be updated to reflect these changes.

Most helpful comment

I think the end state is the right one. But I'd second @nadavrot on avoiding adding the *Ctx versions if it's possible. Since there are a ton of tests to port anyways, I worry that you'll find yourself spending a ton of time swizzling names :-p. (Sometimes you can do these migrations quickly with sed but if not it's a lot of manual pain).

All 4 comments

@gcatron Thanks for doing this work. I have a question about the migration plan. Maybe I missed somethings. Would it make sense to start the process by adding the ctx parameter to run? I think that if you start by moving the ctx parameter to run you won't need to add the new executeCtx and compileCtx APIs.

We probably could get away without compileCtx for sure. My reasoning for adding executeCtx was to maintain an unbroken execution flow. I think the runtime allocation logic will need to reside in the execute method implementations as opposed to run. (would be happy to be wrong on that point)

I think the end state is the right one. But I'd second @nadavrot on avoiding adding the *Ctx versions if it's possible. Since there are a ton of tests to port anyways, I worry that you'll find yourself spending a ton of time swizzling names :-p. (Sometimes you can do these migrations quickly with sed but if not it's a lot of manual pain).

Documenting a conversation @opti-mix and I just had. A candidate API exposed by the CompiledFunction,
Similar to JUnit beforeClass afterClass before after:
setupRuns x1
beforeRun, afterRun xN
tearDownRuns x1

setupRuns would be run once and setup everything that can be done for all runs.
before/afterRun would be what is needed on a per run basis.
tearDownRuns would cleanup.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

wayneshawn picture wayneshawn  路  3Comments

artemrakhov-glow picture artemrakhov-glow  路  4Comments

tlepley-cadence picture tlepley-cadence  路  4Comments

opti-mix picture opti-mix  路  4Comments

qcolombet picture qcolombet  路  5Comments