Fix memory leak

[inliquid]

Issue #, if available:
#78

Description of changes:
The reason for the issue is that GC not run due to cooperative nature of Go runtime in Wasm.

This is fixed by explicitly yielding control with runtime.Gosched. Another option is to force run of GC. It also works, however performance is worse, apparently because it's not necessary to have whole GC cycle on every call to Free. It's enough to make sure that it will run eventually.

I also removed unnecessary call to Free on memory managed by Wasm module itself. This is not needed in any case:

• On Big Go it's a no-op since corresponding map won't contain entry for the given pointer
• On TinyGo it also makes no sense since memory is allocated by module itself, so it's GC area of responsibility, and outside call to Free will either be no-op or lead to further difficult-to-debug memory problems

Last point also reduces overall latency by few microseconds which could have been counted in highly loaded systems.

@knqyf263 I didn't add benchmark tests which I used when reproducing #78, but if you think they make any sense, I can add BenchmarkGreetMorning and BenchmarkGreetEvening similar to the code in the above issue. Not sure if it's ok to run them during test phase in CI/CD. It will need a couple of minutes to complete and should fail because of OOM if original issue not fixed.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[LukaGiorgadze]

Interesting, how can we reproduce memory leak so observe it?

[inliquid]

@LukaGiorgadze please see original issue, I provided steps to reproduce.

[rraymondgh]

I've been working on using AssemblyScript Wasm compiler. This I have working with go-plugins, however is far more sensitive to memory leaks.

I agree these two changes reduce memory leakage. This is another issue that is causing memory leaks. The main cause of memory leaks is

go-plugin/wasm/host.go

Line 34 in c156889

results, err := malloc.Call(ctx, l)

This malloc is never freed. I couldn't work out where to do free, it can't be done WriteMemory() and cannot be done in callers of WriteMemory(). In AssemblyScript I have resolved within implementation of malloc() and free()

import { nanosecond } from "./env";

export namespace pluginmem {
  let mallocCnt = 0;
  let freeCnt = 0;
  let freeRequestCnt = 0;
  let lastCheck:i64 = 0;
  let collectCnt = 0;
  type unpinMapT = Map<i64, usize>;
  let pinnedMap: unpinMapT | null = null;

  export function malloc(size: usize): usize {
    mallocCnt += 1;
    if (mallocCnt - freeCnt > 100 && lastCheck < nanosecond()) {
      lastCheck = nanosecond() + 10 ** 7;
      trace(`malloc[${mallocCnt}] and free[${freeCnt}] out of step`);
    }
    const ptr = __new(size, 0);
    __pin(ptr);

    const n = nanosecond();
    if (pinnedMap == null) {
      pinnedMap = new Map<i64, usize>();
    }
    (<unpinMapT>pinnedMap).set(n, ptr);

    return ptr;
  }

  export function free(ptr: usize): void {
    const n = nanosecond();
    freeRequestCnt += 1;
    const keys = (<unpinMapT>pinnedMap).keys();
    const ptrs = (<unpinMapT>pinnedMap).values();
    for (let i = 0; i < keys.length; i++) {
      if (ptrs[i] == ptr || keys[i] < n - 10**4) {
        freeCnt += 1;
        __unpin(ptrs[i]);
        (<unpinMapT>pinnedMap).delete(keys[i]);
        collectCnt += 1;
      }
    }
    if (collectCnt > 50) {
      collectCnt = 0;
      __collect();
    }
  }
}

This effectively gives memory allocation a TTL. A call to free() will return any memory allocation that has expired TTL. I have two other changes on a branch

• flexibility to define location of Wasm package (allows) customisations of host.go etc as a command line parameter
• removal of types/known

Clearly the second change is more significant. I have not done enough use scenarios to generate a PR yet. This comment is really support for this PR. My testing and usage shows it is good. There is more to do to reduce memory leaks (allocations that are not returned). I will raise at least 1 more PR when I have run more usage scenarios.

[knqyf263]

Thanks. I haven’t had time to test it, but the changes look fine.

[LukaGiorgadze]

@knqyf263 Approximately in 1 month I'll become active contributor of this project, I'm using it for my open-source project. Thanks for keeping it live.