Skip to content

Commit

Permalink
Every require is blocking and requiring the sys module over and over …
Browse files Browse the repository at this point in the history 
…and over again just makes no sense + it hurt performance.. Not to mention.. that it's already included.
  • Loading branch information
@3rd-Eden
3rd-Eden committed Sep 17, 2010
1 parent a387982 commit c6b1765
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/socket.io/listener.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Listener = module.exports = function(server, options){
}
},
log: function(message){
require('sys').log(message);
sys.log(message);
}
}, options);

Expand Down

6 comments on commit c6b1765

@aheckmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious what the perfect hit was on this since require returns the previously required and cached object internally.

@tj
Copy link
Contributor

@tj tj commented on c6b1765 Sep 17, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its pretty bad, loadModule() still has to stat around to resolve the module id I think

@aheckmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eeww

@tj
Copy link
Contributor

@tj tj commented on c6b1765 Sep 18, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deceiving :D that is why in express's view engine i do:

var engine = cache[ext] || (cache[ext] = require(ext))

@aheckmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice I've always required at load time to skip the extra fn calls during execution but didn't know what the perf hit would actually look like.

@3rd-Eden
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well now that I think of it, it would actually be better to use console.log instead of sys.log for logging data. I remember a conversation with ry about the sys module that he is planning to phase out of Node.js. As console.log is already globally available through in Node.js it would probably be more wise to use that instead.. Any thought on that?

Please sign in to comment.