diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c665857..b2d9ee4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ bugs in actually-released versions. ## 3.7 - in development +- Load job compiler scripts on demand, rather than caching them all at startup of the Manager. This makes it simpler to create & test scripts, as the Manager no longer has to be restarted after every update. + ## 3.6 - released 2024-12-01 - Change the name of the add-on from "Flamenco 3" to just "Flamenco". diff --git a/cmd/flamenco-manager/main.go b/cmd/flamenco-manager/main.go index 95f46842..71ee0e6d 100644 --- a/cmd/flamenco-manager/main.go +++ b/cmd/flamenco-manager/main.go @@ -166,7 +166,7 @@ func runFlamencoManager() bool { defer persist.Close() timeService := clock.New() - compiler, err := job_compilers.Load(timeService) + compiler, err := job_compilers.New(timeService) if err != nil { log.Fatal().Err(err).Msg("error loading job compilers") } diff --git a/internal/manager/job_compilers/job_compilers.go b/internal/manager/job_compilers/job_compilers.go index be9a2fda..d878381a 100644 --- a/internal/manager/job_compilers/job_compilers.go +++ b/internal/manager/job_compilers/job_compilers.go @@ -26,13 +26,13 @@ import ( var ErrJobTypeUnknown = errors.New("job type unknown") var ErrJobTypeBadEtag = errors.New("job type etag does not match") -// Service contains job compilers defined in JavaScript. +// Service can load & run job compilers. type Service struct { - compilers map[string]Compiler // Mapping from job type name to the job compiler of that type. - registry *require.Registry // Goja module registry. + registry *require.Registry // Goja module registry. timeService TimeService - // mutex protects 'compilers' from race conditions. + // mutex ensures only one job compiler runs at a time, and protects + // 'registry' from race conditions. mutex *sync.Mutex } @@ -56,20 +56,15 @@ type TimeService interface { Now() time.Time } -// Load returns a job compiler service with all JS files loaded. -func Load(ts TimeService) (*Service, error) { +// New returns a job compiler service. +func New(ts TimeService) (*Service, error) { initFileLoader() service := Service{ - compilers: map[string]Compiler{}, timeService: ts, mutex: new(sync.Mutex), } - if err := service.loadScripts(); err != nil { - return nil, err - } - staticFileLoader := func(path string) ([]byte, error) { content, err := loadFileFromAnyFS(path) if err == os.ErrNotExist { @@ -152,11 +147,8 @@ func (s *Service) Compile(ctx context.Context, sj api.SubmittedJob) (*AuthoredJo func (s *Service) ListJobTypes() api.AvailableJobTypes { jobTypes := make([]api.AvailableJobType, 0) - // Protect access to s.compilers. - s.mutex.Lock() - defer s.mutex.Unlock() - - for typeName := range s.compilers { + compilers := loadScripts() + for typeName := range compilers { compiler, err := s.compilerVMForJobType(typeName) if err != nil { log.Warn().Err(err).Str("jobType", typeName).Msg("unable to determine job type settings") diff --git a/internal/manager/job_compilers/job_compilers_test.go b/internal/manager/job_compilers/job_compilers_test.go index dae5f8e1..07387bab 100644 --- a/internal/manager/job_compilers/job_compilers_test.go +++ b/internal/manager/job_compilers/job_compilers_test.go @@ -66,7 +66,7 @@ func mockedClock(t *testing.T) clock.Clock { func TestSimpleBlenderRenderHappy(t *testing.T) { c := mockedClock(t) - s, err := Load(c) + s, err := New(c) require.NoError(t, err) // Compiling a job should be really fast. @@ -142,7 +142,7 @@ func TestSimpleBlenderRenderHappy(t *testing.T) { func TestSimpleBlenderRenderWithScene(t *testing.T) { c := mockedClock(t) - s, err := Load(c) + s, err := New(c) require.NoError(t, err) // Compiling a job should be really fast. @@ -177,7 +177,7 @@ func TestSimpleBlenderRenderWithScene(t *testing.T) { func TestJobWithoutTag(t *testing.T) { c := mockedClock(t) - s, err := Load(c) + s, err := New(c) require.NoError(t, err) // Compiling a job should be really fast. @@ -206,7 +206,7 @@ func TestJobWithoutTag(t *testing.T) { func TestSimpleBlenderRenderWindowsPaths(t *testing.T) { c := mockedClock(t) - s, err := Load(c) + s, err := New(c) require.NoError(t, err) // Compiling a job should be really fast. @@ -270,7 +270,7 @@ func TestSimpleBlenderRenderWindowsPaths(t *testing.T) { func TestSimpleBlenderRenderOutputPathFieldReplacement(t *testing.T) { c := mockedClock(t) - s, err := Load(c) + s, err := New(c) require.NoError(t, err) // Compiling a job should be really fast. @@ -317,7 +317,7 @@ func TestSimpleBlenderRenderOutputPathFieldReplacement(t *testing.T) { func TestEtag(t *testing.T) { c := mockedClock(t) - s, err := Load(c) + s, err := New(c) require.NoError(t, err) // Etags should be computed when the compiler VM is obtained. @@ -361,7 +361,7 @@ func TestEtag(t *testing.T) { } func TestComplexFrameRange(t *testing.T) { - s, err := Load(mockedClock(t)) + s, err := New(mockedClock(t)) require.NoError(t, err) // Compiling a job should be really fast. diff --git a/internal/manager/job_compilers/scripts.go b/internal/manager/job_compilers/scripts.go index 92ad4e83..f060ad5b 100644 --- a/internal/manager/job_compilers/scripts.go +++ b/internal/manager/job_compilers/scripts.go @@ -14,10 +14,9 @@ import ( "github.com/rs/zerolog/log" ) -// loadScripts iterates over all JavaScript files, compiles them, and stores the -// result into `s.compilers`. -func (s *Service) loadScripts() error { - compilers := map[string]Compiler{} +// loadScripts loads all Job Compiler JavaScript files and returns them. +func loadScripts() map[string]Compiler { + scripts := map[string]Compiler{} // Collect all job compilers. for _, fs := range getAvailableFilesystems() { @@ -37,21 +36,16 @@ func (s *Service) loadScripts() error { // Merge the returned compilers into the big map, skipping ones that were // already there. for name := range compilersfromFS { - _, found := compilers[name] + _, found := scripts[name] if found { continue } - compilers[name] = compilersfromFS[name] + scripts[name] = compilersfromFS[name] } } - // Assign the new set of compilers in a thread-safe way. - s.mutex.Lock() - defer s.mutex.Unlock() - s.compilers = compilers - - return nil + return scripts } // loadScriptsFrom iterates over files in the root of the given filesystem, @@ -156,19 +150,21 @@ func newGojaVM(registry *require.Registry) *goja.Runtime { // compilerVMForJobType returns a Goja *Runtime that has the job compiler script // for the given job type loaded up. func (s *Service) compilerVMForJobType(jobTypeName string) (*VM, error) { - program, ok := s.compilers[jobTypeName] + // TODO: only load the one necessary script, and not everything. + scripts := loadScripts() + script, ok := scripts[jobTypeName] if !ok { return nil, ErrJobTypeUnknown } runtime := newGojaVM(s.registry) - if _, err := runtime.RunProgram(program.program); err != nil { + if _, err := runtime.RunProgram(script.program); err != nil { return nil, err } vm := &VM{ runtime: runtime, - compiler: program, + compiler: script, } if err := vm.updateEtag(); err != nil { return nil, err diff --git a/internal/manager/job_compilers/scripts_test.go b/internal/manager/job_compilers/scripts_test.go index f7f7ec7f..8183859b 100644 --- a/internal/manager/job_compilers/scripts_test.go +++ b/internal/manager/job_compilers/scripts_test.go @@ -65,6 +65,13 @@ func BenchmarkLoadScripts_fromDisk(b *testing.B) { } } +func BenchmarkLoadScripts(b *testing.B) { + zerolog.SetGlobalLevel(zerolog.Disabled) + for i := 0; i < b.N; i++ { + loadScripts() + } +} + // keys returns the set of keys of the mapping. func keys[K comparable, V any](mapping map[K]V) map[K]bool { keys := map[K]bool{}