Search code examples
javascriptnode.jswinston

What are the benefits of adding new methods on a derived class rather than the class itself?


for personal learning purposes, I'm trying to understand Winston's package design structure and the purpose behind each of its modules but I can't figure this one out.

In Winstons package, there is the core Logger class in the logger.js module which implements the main functionality of the logger and provides some public methods such as the logger.log method. It also implements the transform stream methods for internal use.

Then there is a derived class in create-logger.js module called DerivedLogger that extends the Logger class and it seems that its sole purpose is to add optimized level methods to the loggers prototype. This DerivedLogger class is then instantiated and exported in a factory function at the bottom of the module.

My question is, Why was the DerivedLogger class needed? would there be any difference in performance if those level methods were instead added on the Logger class prototype itself and then have the factory function instantiate the Logger class directly? the only reason I could think of is that perhaps the DerivedLogger class is added for modularity purposes only? can someone help me understand the reason?

Thanks!


Solution

  • This one was very interesting, thanks for pointing it out!


    In short: It has nothing to do with code-structure, it's a performance optimization. The comment states as much:

    Create a new class derived logger for which the levels can be attached to the prototype of. This is a V8 optimization that is well know to increase performance of prototype functions.

    Personally, I think this needs citation (and I wouldn't accept it in a code-review). Luckily, I think I found the "optimization" that the author was talking about:

    This article by Mathias (a Google engineer that works on V8) talks about speeding up JavaScript execution with correct usage of prototype. The article has a lot of details and is really worth the read if you're learning.

    The optimization found in Winston boils down to this:

    The getAttribute() method is found on the Element.prototype. That means each time we call anchor.getAttribute(), the JavaScript engine needs to…

    • check that getAttribute is not on the anchor object itself,
    • check that the direct prototype is HTMLAnchorElement.prototype,
    • assert absence of getAttribute there,
    • check that the next prototype is HTMLElement.prototype,
    • assert absence of getAttribute there as well,
    • eventually check that the next prototype is Element.prototype,
    • and that getAttribute is present there.

    That’s a total of 7 checks! Since this kind of code is pretty common on the web, engines apply tricks to reduce the number of checks necessary for property loads on prototypes.

    This is roughly applicable to Winston as follows:

    • Methods on Classes are defined on the prototype-object of said class
    • Every time a method is called on an instance, the engine needs to find the prototype that the called method is attached to.
    • In doing so, it takes the prototype for the instance-class that you're calling the method on and checks to find the called method on it's prototype
    • If it can't find it (e.g. because the method is inherited), it walks up the prototype-chain to the next class and looks there
    • This continues until either the method is found (and executed sub-sequentially) or the end of the prototype-chain is reached (and an error is thrown).

    By running _setupLevels() in the constructor, the level-methods are directly attached to the prototype of the specific logger-implementations instance. This means that the class-hierarchy can grow arbitrarily large: The prototype-chain lookup only takes 1 step to find the method

    Here is another (simplified) example:

    class A {
        constructor() {
            this.setup();
        }
        testInherit() {
            console.log("Inherited method called");
        }
        setup() {
            this["testDirect"] = () => console.log("Directly attached method called");
        }
    
    }
    
    class B extends A {
        constructor() {
            super();
        }
    }
    
    const test = new B();
    test.testInherit();
    test.testDirect();
    

    If we set a breakpoint after test is instantiated, we see the following:

    enter image description here

    As you can see, the testDirect-method is directly attached to test, while testInherit is multiple levels down.


    I personally think this is bad practice:

    • This might be true now, but it might not be true in the future. If V8 optimizes this internally, the current "optimization" might be significantly slower.
    • The claim that it's an optimization holds no merit without profiling and sources.
    • It's complicated to understand (see this question)
    • Doing this can actually hurt performance. The linked article states as it's conclusion:

    don’t mess with prototypes


    As for modularity: There is something to be said for having a clear base-class for all extension.

    In a stricter language than JavaScript, such a class could offer specific methods that are only meant for extending, which are hidden from the public API for consumers. In this specific case however, Logger would have been fine on its own.