refactor: call tracer only after isEnabled check

[sis0k0]

original PR: #958

[sis0k0]

test

[NathanWalker]

@sis0k0 just curious - couldn't this perhaps just be called in tracer itself to avoid having to have these conditionals all over the codebase? ie:

import { isEnabled } from "tns-core-modules/trace";

export function animationsLog(message: string): void {
    if (isEnabled()) {
      write(message, animationsTraceCategory);
   }
}

[sis0k0]

The disadvantage of having the check inside the tracer itself is that this will result in two function calls - one for animationsLog and one for isEnabled.

[sis0k0]

Also if the check was inside the animationsLog function and we invoked animationsLog("something" + random), we would have the overhead of the string concatenation as well.

[NathanWalker]

Thanks for explanation makes sense :) Perhaps there’s a possibility we could also add something to webpack config to strip these all out in production mode as well to coincide with this pr. Ideally with the whole trace enabler control we should probably have ways that devs can strip them when desired as well since keeping core libs slim and performant as possible is always highest goal (which thank you for attention to that here!).

[sis0k0]

That's a great idea! 10x :)

[NathanWalker]

For anyone that comes across this PR and is also wondering @NathanaelA provided an additional explanation which is very helpful:

---

Performance. Take this example :

if (trace.isEnabled()) {  
  var generateSomeData = doA()+"-"+doB();
  trace.log("We are tracing some data", generateSomeData, moreInfo ? dox() : doy());  
}

By calling trace.isEnabled() and getting back a false it skips:

1. Setting up another call into the trace system.
2. Elimnates doA, doB, and a variable allocation that is NOT needed.
3. Allocating stack space to pass in 3 values
4. evaluating moreInfo
5. eliminate DoX() or doY() calls
6. Cleanup the stack on exit of each of those functions.
   In addition if you use webpack; you should be able to make trace.isEnabled() a nop instruction which would then allow webpack strip out the entire if statement, giving you even more performance as your JS file is smaller. 🙂

It is always recommended to check if you can log before blindly calculating and logging. Logging can be expensive; especially if any of the do?() functions are expensive...

For those who have trace logging enabled in that area to debug something; the ms time used that occurs to run the isEnabled() check is tiny compared to the actual log command. So they don't experience any slow down by doing it that way..

[sis0k0]

Thank you, @NathanWalker, for taking the time to explain this in detail!