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

[service] Enable intra-session mutable state #591

Open
ChrisCummins opened this issue Mar 2, 2022 · 0 comments · May be fixed by #609
Open

[service] Enable intra-session mutable state #591

ChrisCummins opened this issue Mar 2, 2022 · 0 comments · May be fixed by #609
Labels
Enhancement New feature or request RPC

Comments

@ChrisCummins
Copy link
Contributor

ChrisCummins commented Mar 2, 2022

🚀 Feature

Motivation

The CompilationSession interface allows mutable state only for the duration of a single compilation session, but there are cases where we may want to have mutable state that outlives a single session and is shared between sessions over the lifetime of a service. For example, the LLM service implements a BenchmarkFactory class which is an in-memory cache of parsed bitcodes that is shared across all services.

Pitch

Add a new class that encapsulates all intra-session state of a compiler service in a "Context" object:

class CompilerGymServiceContext {
 public:
  // Called first. User setup routines go here.
  [[nodiscard]] virtual grpc::Status init();

  // Called last. User shutdown routines go here.
  [[nodiscard]] virtual grpc::Status shutdown();

  const boost::filesystem::path& workingDirectory() const;
}

Then change the CompilationSession interface so that it takes a reference to this context:

class CompilationSession {
 public:
  CompilationSession(CompilerGymServiceContext& ctx);
}

The helper function for creating and running a CompilerGymService then needs to be parametrized by this context type:

template <typename CompilationSessionType, typename CompilerGymServiceContext = DefaultCompilerGymServiceContext>
[[noreturn]] void createAndRunCompilerGymService(int argc, char** argv, const char* usage) {...}

Importantly, implementing this new context should be optional. We should provide a default implementation for users who don't need to use it. That way, the only breaking change will be if users decided to overload the default constructor for CompilationSession, since we are changing its signature.

Considerations

  • The user needs to make sure that any custom logic added to CompilerServiceContext is thread-safe, since a single context object will be shared by all sessions.

Alternatives

Don't provide a context object, but allow users to run their own shutdown code after createAndRunCompilerGymService() completes. The downside to this is that the user needs to remember to return the appropriate return code from createAndRunCompilerGymService().

@ChrisCummins ChrisCummins added the Enhancement New feature or request label Mar 2, 2022
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Mar 2, 2022
This adds an explicit call to BenchmarkFactory::close() on the global
singleton, since otherwise it may not be called.

Fixes facebookresearch#582.
Issue facebookresearch#591.
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Mar 3, 2022
This adds an explicit call to BenchmarkFactory::close() on the global
singleton, since otherwise it may not be called.

Fixes facebookresearch#582.
Issue facebookresearch#591.
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Mar 6, 2022
@ChrisCummins ChrisCummins linked a pull request Mar 6, 2022 that will close this issue
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Mar 6, 2022
This adds an explicit call to BenchmarkFactory::close() on the global
singleton, since otherwise it may not be called.

Fixes facebookresearch#582.
Issue facebookresearch#591.
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request RPC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant