Why is Trim cached?

[magiblot]

I see Trim and other filters were made cached in eb018b8. Would there be any inconvenient to making it non-cached again?

Thank you.

[pinterf]

Hmm. I really don't know what other issue in connection of no-op filter group triggered this commit.
I have to visit back to the history around Aug 29 2019. I agree that it was an unnecessary move regarding Trim.
Did you have memory issues again?

[pinterf]

Yep. This one:
https://forum.doom9.org/showthread.php?p=1883114#post1883114
and the finding
https://forum.doom9.org/showthread.php?p=1883255#post1883255

[magiblot]

Well, I do have high memory usage. I haven't measured the impact, but since my script makes over a hundred Trim instances, I figured out it would help not to cache them.

I remember those doom9 posts. However, in the given example, isn't Splice the one which benefits from caching?

[pinterf]

I'd keep the caching behaviour, it helped a lot. Memory is free but it is not so in your use case. Your script shed light on the dark side of this feature. Both mode would be nice to be supported. Would you accept a new parameter which controls caching? I think, similarly to deciding MT mode (MT_SERIALIZED or MT_MULTI_INSTANCE) upon specific parameter setting, caching (answering DONT_CACHE_ME in cachehint query) can be controlled after parameter parsing.

[magiblot]

Yes, that would be good enough. (It would be even better to be able to control this globally, just like SetFilterMTMode does for MT modes, but that is probably too much work).

[pinterf]

cache=true: 24397 fps, 96 MiB memory
cache=false: 2088 fps, 44 MiB memory

clip=BlankClip(length=1000000000, pixel_type = "RGB32")
clp = clip.ConvertToRGB24
a = clp.Trim(20,-30, cache=false) + clp.Trim(2000,-30, cache=false)
a = a+a+a+a+a+a+a+a+a+a
a = a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a
a = a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a+a
return a

[magiblot]

Okay, you were right.

Thank you!

[pinterf]

And you were right as well with the lower memory consumption.

[pylorak]

Is it known/understood why Trim needs a cache for performance? Trim (and other filters like DeleteFrame and so on) didn't have any caching because it shouldn't help. These filters only "renumber" frames, which is so fast at least in theory that a cache for them only adds memory usage without any performance gains. Now obviously this wasn't the case. But it raises the question if there is some deeper phenomenon that we're not aware yet. It might be better to get to the bottom of that, maybe it can be fixed without adding memory overhead to Trim and its friends.

[magiblot]

Would you find it a good idea to have a NoCache function which returns a non-cached reference to a clip? (The opposite of Cache)

This is a possible implementation (which seems to work for me):

--- a/avs_core/core/cache.cpp
+++ b/avs_core/core/cache.cpp
@@ -48,6 +48,7 @@
 extern const AVSFunction Cache_filters[] = {
   { "Cache", BUILTIN_FUNC_PREFIX, "c[name]s", CacheGuard::Create },
   { "InternalCache", BUILTIN_FUNC_PREFIX, "c[name]s", CacheGuard::Create },
+  { "NoCache", BUILTIN_FUNC_PREFIX, "c", NoCache::Create },
   { 0 }
 };
 
@@ -724,3 +725,43 @@ bool __stdcall CacheGuard::IsCache(const PClip& p)
 {
   return ((p->GetVersion() >= 5) && (p->SetCacheHints(CACHE_IS_CACHE_REQ, 0) == CACHE_IS_CACHE_ANS));
 }
+
+AVSValue __cdecl NoCache::Create(AVSValue args, void*, IScriptEnvironment* env)
+{
+  PClip p = args[0].AsClip();
+  if (auto *cache = dynamic_cast<CacheGuard *>(p.operator->())) {
+    return new NoCache(cache->child);
+  }
+  return p;
+}
+
+PVideoFrame __stdcall NoCache::GetFrame(int n, IScriptEnvironment* env)
+{
+  return child->GetFrame(n, env);
+}
+
+bool __stdcall NoCache::GetParity(int n)
+{
+  return child->GetParity(n);
+}
+
+void __stdcall NoCache::GetAudio(void* buf, int64_t start, int64_t count, IScriptEnvironment* env)
+{
+  return child->GetAudio(buf, start, count, env);
+}
+
+int __stdcall NoCache::SetCacheHints(int cachehints, int frame_range)
+{
+  switch(cachehints)
+  {
+    case CACHE_DONT_CACHE_ME:
+      return 1;
+    default:
+      return child->SetCacheHints(cachehints, frame_range);
+  }
+}
+
+const VideoInfo& __stdcall NoCache::GetVideoInfo()
+{
+  return child->GetVideoInfo();
+}
--- a/avs_core/core/cache.h
+++ b/avs_core/core/cache.h
@@ -124,6 +124,27 @@ private:
     CACHE_25_AUDIO_NONE=4,
     CACHE_25_AUDIO_AUTO=5,
   };
+  friend class NoCache;
+};
+
+class NoCache : public IClip
+{
+  PClip child;
+
+  NoCache(PClip &aChild) :
+    child(aChild)
+  {
+  }
+
+public:
+
+  PVideoFrame __stdcall GetFrame(int n, IScriptEnvironment* env);
+  bool __stdcall GetParity(int n);
+  void __stdcall GetAudio(void* buf, int64_t start, int64_t count, IScriptEnvironment* env);
+  int __stdcall SetCacheHints(int cachehints,int frame_range);
+  const VideoInfo& __stdcall GetVideoInfo();
+
+  static AVSValue __cdecl Create(AVSValue args, void*, IScriptEnvironment* env);
 };
 
 #endif  // __Cache_H__

[pinterf]

Sorry for not reacting immediately.
So by using dynamic_cast you can check whether the child is a CacheGuard or not. And if it is a cache filter, you make a shortcut and call the original cached child with this new dedicated NO_CACHE filter?
I wonder how the filter graph is shown.

[magiblot]

DumpFilterGraph does not seem to show Cache instances, so here are backtraces instead.

Given this script:

SetFilterMTMode("Trim", MT_MULTI_INSTANCE, force=true) # To see what would happen with a filter using MTGuards

BlankClip()
Trim(1, 20)

You get this backtrace:

#0  SingleFrame::GetFrame(int, IScriptEnvironment*)
#1  Trim::GetFrame(int, IScriptEnvironment*)
#2  MTGuard::GetFrame(int, IScriptEnvironment*)
#3  Cache::GetFrame(int, IScriptEnvironment*)
#4  CacheGuard::GetFrame(int, IScriptEnvironment*)
#5  avs_get_frame(AVS_Clip*, int)

And given this script:

SetFilterMTMode("Trim", MT_MULTI_INSTANCE, force=true)

BlankClip()
Trim(1, 20).NoCache

You get this backtrace:

#0  SingleFrame::GetFrame(int, IScriptEnvironment*)
#1  Trim::GetFrame(int, IScriptEnvironment*)
#2  MTGuard::GetFrame(int, IScriptEnvironment*)
#3  NoCache::GetFrame(int, IScriptEnvironment*)
#4  avs_get_frame(AVS_Clip*, int)