{"action":"created","issue":{"url":"https://api.github.com/repos/Microsoft/TypeScript/issues/9925","repository_url":"https://api.github.com/repos/Microsoft/TypeScript","labels_url":"https://api.github.com/repos/Microsoft/TypeScript/issues/9925/labels{/name}","comments_url":"https://api.github.com/repos/Microsoft/TypeScript/issues/9925/comments","events_url":"https://api.github.com/repos/Microsoft/TypeScript/issues/9925/events","html_url":"https://github.com/Microsoft/TypeScript/issues/9925","id":167338867,"number":9925,"title":"Optimise __extends","user":{"login":"Rycochet","id":290714,"avatar_url":"https://avatars.githubusercontent.com/u/290714?v=3","gravatar_id":"","url":"https://api.github.com/users/Rycochet","html_url":"https://github.com/Rycochet","followers_url":"https://api.github.com/users/Rycochet/followers","following_url":"https://api.github.com/users/Rycochet/following{/other_user}","gists_url":"https://api.github.com/users/Rycochet/gists{/gist_id}","starred_url":"https://api.github.com/users/Rycochet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Rycochet/subscriptions","organizations_url":"https://api.github.com/users/Rycochet/orgs","repos_url":"https://api.github.com/users/Rycochet/repos","events_url":"https://api.github.com/users/Rycochet/events{/privacy}","received_events_url":"https://api.github.com/users/Rycochet/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-07-25T10:52:09Z","updated_at":"2016-07-25T15:53:58Z","closed_at":null,"body":"I was trying to find places to optimise a large project, and realised that the longest running function was actually the __extends call, which prompted me to look at the current code.\r\n\r\n# Current code\r\n\r\nTwo things immediately jumped out at me with the current code:\r\n```javascript\r\nvar __extends = (this && this.__extends) || function (d, b) {\r\n    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];\r\n    function __() { this.constructor = d; }\r\n    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());\r\n};\r\n```\r\n\r\n# Problems\r\n\r\n## Potentially unused code (pre ES5)\r\n\r\nThe first problem was that the `__()` function was being created, when it might not be used (though chances of that are probably very small).\r\n\r\nThe Solution would be to move the check into an if/else. Related to this is that `null` doesn't have any properties, so the property copying should be in the \"not null\" section.\r\n\r\n```javascript\r\nvar __extends = (this && this.__extends) || function (d, b) {\r\n\tif (b===null) {\r\n\t\td.prototype = Object.create(b);\r\n\t} else {\r\n\t\tfor (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];\r\n\t\tvar __=function() { this.constructor = d; };\r\n\t\t__.prototype = b.prototype;\r\n\t\td.prototype = new __();\r\n\t}\r\n};\r\n```\r\n\r\n## Inefficient iteration (for ES5+)\r\n\r\nThe second problem is purely performance related with the property copying. `for(var p in b)` will iterate over the entire prototype tree, but `if(b.hasOwnProperty(p))` will then skip any that aren't directly on the object/class itself. (see [Enumerability and ownership of properties](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties))\r\n\r\nThe solution would be to use `Object.keys(b)` and iterating an array, which will work only on the object/class itself (and not it's prototype tree), it also automatically skips the same keys we'd be skipping anyway - so taking most of what we're wanting to the browser / native code instead of JS.\r\n\r\nUnlike `for...in`, Object.keys fails when called on `null`, `undefined` or any other non-Object, so this requires some extra code to help prevent that happening.\r\n\r\nFrom [here](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Object/keys#Specifications) we see that `Object.keys()` is an ES5+ function.\r\n\r\nSuggested code:\r\n\r\n```javascript\r\nvar __extends = (this && this.__extends) || function (d, b) {\r\n\tif (b instanceof Object) {\r\n\t\tObject.keys(b).forEach(function(p){\r\n\t\t\td[p] = b[p];\r\n\t\t});\r\n \t\tvar __=function() { this.constructor = d; };\r\n\t\t__.prototype = b.prototype;\r\n\t\td.prototype = new __();\r\n\t} else {\r\n\t\td.prototype = Object.create(null);\r\n\t}\r\n};\r\n```\r\n\r\n# Results\r\n\r\nIt's very hard to get reliable timings for the code on Chrome Profiling, but on average this looks like roughly a 10% speedup at startup for my project (38 classes in a nested tree up to six deep). There is no major benefit to this, though it does make the code slightly \"safer\".\r\n\r\nThe ES5+ code could potentially cause different behaviour if a class tries to inherit from a non-class (such as a number or string), though I'm not sure that is possible to begin with..\r\n\r\n# Language Feature Checklist\r\n\r\n* Syntactic: No changes as this is a compiler change.\r\n* Semantic: No changes as the final code does the same thing when used as intended (see Results above).\r\n* Emit: Output code will change depending on ES version needed, both versions are slightly larger than the original.\r\n* Compatibility: Two versions, one for pre-ES5, most efficient for ES5+\r\n* Other: There is already an option to prevent __extends being output (which only happens if needed anyway). This could potentially hit older build scripts that manually search & replace the old code."},"comment":{"url":"https://api.github.com/repos/Microsoft/TypeScript/issues/comments/234995858","html_url":"https://github.com/Microsoft/TypeScript/issues/9925#issuecomment-234995858","issue_url":"https://api.github.com/repos/Microsoft/TypeScript/issues/9925","id":234995858,"user":{"login":"yortus","id":1252361,"avatar_url":"https://avatars.githubusercontent.com/u/1252361?v=3","gravatar_id":"","url":"https://api.github.com/users/yortus","html_url":"https://github.com/yortus","followers_url":"https://api.github.com/users/yortus/followers","following_url":"https://api.github.com/users/yortus/following{/other_user}","gists_url":"https://api.github.com/users/yortus/gists{/gist_id}","starred_url":"https://api.github.com/users/yortus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yortus/subscriptions","organizations_url":"https://api.github.com/users/yortus/orgs","repos_url":"https://api.github.com/users/yortus/repos","events_url":"https://api.github.com/users/yortus/events{/privacy}","received_events_url":"https://api.github.com/users/yortus/received_events","type":"User","site_admin":false},"created_at":"2016-07-25T15:53:58Z","updated_at":"2016-07-25T15:53:58Z","body":"Just a hypothetical....\r\n\r\nIf you had a project that creates 1,000,000 instances across 10 different classes, then `__extends` would be called only 10 times and the GC pressure of those 10 `__()` instances (say 9 of which are necessary anyway) would be utterly negligable, and even the tiniest tweak to the code that repeats 1 million times would yield far greater gains.\r\n\r\nOTOH, if your project has `__extends` on a hot path, then its more like creating 1 instance each of 1,000,000 class expressions. That just seems a wildly inefficient approach to using classes, and again the gains would be far greater in factoring out the class expressions to reduce the number of `__extends` calls by several orders of magnitude.\r\n"}}
{"actor":{"display_login":"yortus"}}