From e48fa4cc5fa06ae954c7fc240a829bd29f70ac5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Mon, 10 Feb 2025 12:05:52 +0100 Subject: [PATCH] Manager: load job compiler scripts on demand, instead of at startup The Manager now loads the JavaScript files for job types on demand, instead of caching them in memory at startup. This will make certain calls a bit less performant, but in practice this is around the order of a millisecond so it shouldn't matter much. Fixes: #104349 --- CHANGELOG.md | 2 ++ cmd/flamenco-manager/main.go | 2 +- .../manager/job_compilers/job_compilers.go | 24 ++++++----------- .../job_compilers/job_compilers_test.go | 14 +++++----- internal/manager/job_compilers/scripts.go | 26 ++++++++----------- .../manager/job_compilers/scripts_test.go | 7 +++++ 6 files changed, 36 insertions(+), 39 deletions(-) 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{}