1

I'm reading through Winston's code base and there was a comment in their DerivedLogger class line 22 that says:

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 known to increase performance of prototype functions.

From what I gather here, they are saying that adding a new class (DerivedLogger) which adds prototype methods is a well known form of V8 optimization? how is it different from just adding the methods to the prototype of the class Logger without having to create a new class? can someone help me understand this concept or correct me if I am misunderstanding the comment here? Thank you!

try-catch-finally
  • 6,720
  • 6
  • 37
  • 64
tito.300
  • 417
  • 4
  • 14
  • Just for the record: [this is how this code looked like before a refactoring](https://github.com/winstonjs/winston/blob/1146e645df303e9645521322019f8d99ed66c18c/lib/winston/create-logger.js#L20) in 852337e51f0df03fec85f955aa6b4e9833b3bd00. Before that change it really changed the prototype. As in the [current version, you linked](https://github.com/winstonjs/winston/blob/64344d12eace394d9e7c8bf926f9f2663efc8640/lib/winston/create-logger.js) this comment is simply wrong. – try-catch-finally Dec 26 '18 at 20:05
  • Possible duplicate of [Advantages of using prototype, vs defining methods straight in the constructor?](https://stackoverflow.com/questions/4508313/advantages-of-using-prototype-vs-defining-methods-straight-in-the-constructor) – try-catch-finally Dec 26 '18 at 20:08
  • Note, the duplicate answer [https://stackoverflow.com/a/4508573/1078886](https://stackoverflow.com/a/4508573/1078886) contains a link to a JSperf comparison: http://jsperf.com/class-comparison – try-catch-finally Dec 26 '18 at 20:10
  • @try-catch-finally okay that makes sense. my question now is, was there a need to create a new class to add those methods? Couldn’t we add those methods on the Logger class itself? Or would that make a difference besides code readability and modularity? Thanks! – tito.300 Dec 26 '18 at 20:47

1 Answers1

2

Well, that's an interesting one.

Foreword

Without any details on that "well known" "increase [on] performance" we can only speculate what was meant.

History of the logger

When I first saw your question and the code I realized the code comment must be outdated.

class DerivedLogger extends Logger {
  /**
   * 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.
   * @param {!Object} options - Options for the created logger.
   */
  constructor(options) {
    super(options);
    this._setupLevels();
  }
  // ...
}

module.exports = (opts = { levels: config.npm.levels }) => (
  new DerivedLogger(opts)
);

Source

It was wrong in one way, since, via the _setupLevels() call in the constructor, the methods are defined on the instance, not on the prototype. See here or here for details on that topic.

So I dug through the history to find the first occurence of that comment with unchanged code ...

This is what the original code looked like when the above comment was added:

// Create a new instance of a winston Logger. Creates a new
// prototype for each instance.
module.exports = function (opts) {
  // ...

  //
  // Create a new prototypal 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.
  //
  function DerivedLogger(options) { Logger.call(this, options); }
  util.inherits(DerivedLogger, Logger);

  // ...

  DerivedLogger.prototype[level] = function (msg) {

Source

The current code changed in another way: the DerivedLogger is no longer created with every logger instance but only once, when the module is loaded.

Analysis

Until here I did not realize that the Winston authors created new prototypes in the logger's create function:

// Create a new instance of a winston Logger. Creates a new
// prototype for each instance.
//
module.exports = function (opts) {

Source

So, when a new logger is to be created, not only an instance is made, but a whole new prototype is created too.

               [Logger]  (A)
                   ^
                   |
         +---------+--------+
         |                  |
 [DerivedLogger #1] [DerivedLogger #2]  (B)
         |                  |
      logger #1          logger #2

Derived loggers are not reused.

Conclusion

The original intent definitively was to prevent modification/pollution of Logger (A) whenever a new logger instance is created.

Though creating logger methods on a prototype to prevent wasting memory in repeated instance methods (see the linked questions at the beginning) seems to be thwarted by the repeated creation of new prototypes.

I even believe the performance gained by creating a prototype holding log methods over defining them directly for the instance is swallowed by the creation of the prototype object.

However, I'm not 100% confident that the discussed interpretation was the original intent and am open for corrections and clarifications.

Bonus

(I found this during my research and probably unrelated to the above Winston code.)

Since it bothered me that the original author claimed defining methods on the prototype would optimize things for V8, I started searching for updates on this topic and I found an article by V8 developer Mathias Bynens: JavaScript engine fundamentals: optimizing prototypes.

He's discussing how most Javascript engines (not only V8!) internally store objects and how they handle property access. You might also want to read another article by him on Shape objects.

I won't fully recap that in detail here, though there seems to be a unique detail to V8 how accesses along the prototype chain is handled:

V8 treats prototype shapes specially for this purpose. Each prototype has a unique shape that is not shared with any other objects (specifically not with other prototypes), and each of these prototype shapes has a special ValidityCell associated with it.
This ValidityCell is invalidated whenever someone changes the associated prototype or any prototype above it.
[...]
The next time the Inline Cache is hit, the engine has to check the shape of the instance and the ValidityCell. If it’s still valid, the engine can reach out directly to the Offset on the Prototype, skipping the additional lookups.

(Bold text by me.)

So, unique to V8 seems to be the fact, that they track whether or not the prototype is still "in shape". This allows V8 to reduce checks involved in the prototype chain processing.

try-catch-finally
  • 6,720
  • 6
  • 37
  • 64
  • Thanks for the detailed answer. The bonus part was really helpful. So I understand that they needed a new Derived class to allow for modification in the prototype for each new DerivedLogger instance without having to create or touch the extended Loggers prototype? Also When you said "the DerivedLogger is no longer created with every logger instance but only once, when the module is loaded." Are you sure this is the case? because there is a factory function at the bottom which creates a new instances every time createLogger is called and not just once. – tito.300 Dec 28 '18 at 18:39
  • Yes, the instance is created by the factory function, but **not the derived logger class/prototype**. Though that is what they originally did and what still remains in the comment: `"Create a new class derived logger"` (which is no longer true). – try-catch-finally Dec 28 '18 at 19:23
  • `"So I understand that they needed a new Derived class to allow for modification in the prototype for each new DerivedLogger instance without having to create or touch the extended Loggers prototype"` - Yes. Though I strongly doubt that this had any performance gain compared to (repeated) definition on the instance object (like in the two linked questions). – try-catch-finally Dec 28 '18 at 19:26
  • Ok I see. Yea, so I just am wondering why not just add the level methods to the `Logger` prototype itself? why a new class `DerivedLogger` was needed? The only reason I can think of is modularity because in both cases they still have the same performance... but maybe I'm wrong and someone can point out the reason. – tito.300 Dec 28 '18 at 20:08