{"action":"created","comment":{"url":"https://api.github.com/repos/dotnet/corefx/pulls/comments/35260727","id":35260727,"diff_hunk":"@@ -676,38 +676,191 @@ private static IEnumerable<TSource> TakeWhileIterator<TSource>(IEnumerable<TSour\n             }\n         }\n \n+        // Just in case somebody does something weird with the ordering of enumerator\n+        // disposal, this wraps an enumerator that has been used, but not disposed,\n+        // and disposes it when this is.\n+        // If an exception had been found while skipping, this\n+        // throws it on MoveNext.\n+        private sealed class DeadEnumeratorWrapper<TSource> : IEnumerator<TSource>\n+        {\n+            private readonly IEnumerator<TSource> _corpse;\n+            private ExceptionDispatchInfo _exception;\n+\n+            internal DeadEnumeratorWrapper(IEnumerator<TSource> corpse)\n+                : this(corpse, null)\n+            {\n+            }\n+\n+            internal DeadEnumeratorWrapper(IEnumerator<TSource> corpse, ExceptionDispatchInfo exception)\n+            {\n+                _corpse = corpse;\n+                _exception = exception;\n+            }\n+\n+            public TSource Current\n+            {\n+                get { return default(TSource); }\n+            }\n+\n+            object IEnumerator.Current\n+            {\n+                get { return Current; }\n+            }\n+\n+            public bool MoveNext()\n+            {\n+                if (_exception != null)\n+                {\n+                    ExceptionDispatchInfo ex = _exception;\n+                    _exception = null;\n+                    ex.Throw();\n+                }\n+                return false;\n+            }\n+\n+            public void Dispose()\n+            {\n+                _corpse.Dispose();\n+            }\n+\n+            public void Reset()\n+            {\n+                throw new NotSupportedException();\n+            }\n+        }\n+\n         public static IEnumerable<TSource> Skip<TSource>(this IEnumerable<TSource> source, int count)\n         {\n             if (source == null) throw Error.ArgumentNull(\"source\");\n-            return SkipIterator<TSource>(source, count);\n+            return new SkipIterator<TSource>(source, count);\n         }\n \n-        private static IEnumerable<TSource> SkipIterator<TSource>(IEnumerable<TSource> source, int count)\n+        private sealed class SkipIterator<TSource> : IEnumerable<TSource>\n         {\n-            using (IEnumerator<TSource> e = source.GetEnumerator())\n+            private readonly IEnumerable<TSource> _source;\n+            private readonly int _count;\n+\n+            public SkipIterator(IEnumerable<TSource> source, int count)\n             {\n-                while (count > 0 && e.MoveNext()) count--;\n-                if (count <= 0)\n+                _source = source;\n+                _count = count > 0 ? count : 0;\n+            }\n+\n+            public IEnumerator<TSource> GetEnumerator()\n+            {\n+                IEnumerator<TSource> sourceEnumerator = _source.GetEnumerator();\n+                try\n                 {\n-                    while (e.MoveNext()) yield return e.Current;\n+                    for (int count = _count; count != 0; --count)\n+                    {\n+                        // It should be harmless to have the recipient call MoveNext again.\n+                        // but we guard against strange behaviour from shortcuts in the enumerator\n+                        // It should also be harmless to dispose the enumerator here if we've\n+                        // exhausted it, but we guard against strange behaviour in either the\n+                        // enumerator or the caller.\n+                        if (!sourceEnumerator.MoveNext()) return new DeadEnumeratorWrapper<TSource>(sourceEnumerator);\n+                    }\n+                    return sourceEnumerator;\n+                }\n+                catch (Exception ex)\n+                {\n+                    // Force the exception to happen on a MovedNext from the caller, rather than\n+                    // here.\n+                    return new DeadEnumeratorWrapper<TSource>(sourceEnumerator, ExceptionDispatchInfo.Capture(ex));\n                 }\n             }\n+\n+            IEnumerator IEnumerable.GetEnumerator()\n+            {\n+                return GetEnumerator();\n+            }\n         }\n \n         public static IEnumerable<TSource> SkipWhile<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)\n         {\n             if (source == null) throw Error.ArgumentNull(\"source\");\n             if (predicate == null) throw Error.ArgumentNull(\"predicate\");\n-            return SkipWhileIterator<TSource>(source, predicate);\n+            return new SkipWhileIterator<TSource>(source, predicate);\n         }\n \n-        private static IEnumerable<TSource> SkipWhileIterator<TSource>(IEnumerable<TSource> source, Func<TSource, bool> predicate)\n+        private sealed class SkipWhileIterator<TSource> : IEnumerable<TSource>, IEnumerator<TSource>\n         {\n-            bool yielding = false;\n-            foreach (TSource element in source)\n+            private readonly IEnumerable<TSource> _source;\n+            private readonly Func<TSource, bool> _predicate;\n+            private readonly int _threadId = Environment.CurrentManagedThreadId;\n+            private IEnumerator<TSource> _enumerator;\n+            private int _state;\n+            private TSource _current;\n+\n+            internal SkipWhileIterator(IEnumerable<TSource> source, Func<TSource, bool> predicate)\n+            {\n+                _source = source;\n+                _predicate = predicate;\n+            }\n+\n+            public IEnumerator<TSource> GetEnumerator()\n+            {\n+                // Don't short-circuit. These are almost always both true, so don't \"save\"\n+                // time by branching after a very likely case.","path":"src/System.Linq/src/System/Linq/Enumerable.cs","position":148,"original_position":148,"commit_id":"3658c688bfe585a0fadb14b1b04ecca7c466ab2e","original_commit_id":"3658c688bfe585a0fadb14b1b04ecca7c466ab2e","user":{"login":"VSadov","id":8218165,"avatar_url":"https://avatars.githubusercontent.com/u/8218165?v=3","gravatar_id":"","url":"https://api.github.com/users/VSadov","html_url":"https://github.com/VSadov","followers_url":"https://api.github.com/users/VSadov/followers","following_url":"https://api.github.com/users/VSadov/following{/other_user}","gists_url":"https://api.github.com/users/VSadov/gists{/gist_id}","starred_url":"https://api.github.com/users/VSadov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/VSadov/subscriptions","organizations_url":"https://api.github.com/users/VSadov/orgs","repos_url":"https://api.github.com/users/VSadov/repos","events_url":"https://api.github.com/users/VSadov/events{/privacy}","received_events_url":"https://api.github.com/users/VSadov/received_events","type":"User","site_admin":false},"body":"_state==0 is generally more common. However there are cases when it is not the case and in those cases the opposite is true.\r\n\r\nI have seen cases (in Roslyn, before we switched most of the APIs to use ImmutableArray) when public object model is based on lazy-initialized IEnumerable wrappers of internal structures. That is the case where it is common for the same enumerable to be enumerated over and over and CurrentManagedThreadId was noticeable in profiles in a form of CPU cache misses. - not sure if it forces some volatile semantics or just because it touches data too far from the iterator instance. That prompted swapping the operands of && in generated code. Old compiler did the threadId compare first, for no particular reason.\r\n\r\nI believe the     stateCheck && threadIdCheck     pattern is a nice compromise that gives up just a little in an optimal scenario (_state == 0) while does not penalize us too much in other scenarios.","created_at":"2015-07-22T20:38:24Z","updated_at":"2015-07-22T20:38:24Z","html_url":"https://github.com/dotnet/corefx/pull/2446#discussion_r35260727","pull_request_url":"https://api.github.com/repos/dotnet/corefx/pulls/2446","_links":{"self":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/comments/35260727"},"html":{"href":"https://github.com/dotnet/corefx/pull/2446#discussion_r35260727"},"pull_request":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/2446"}}},"pull_request":{"url":"https://api.github.com/repos/dotnet/corefx/pulls/2446","id":40551736,"html_url":"https://github.com/dotnet/corefx/pull/2446","diff_url":"https://github.com/dotnet/corefx/pull/2446.diff","patch_url":"https://github.com/dotnet/corefx/pull/2446.patch","issue_url":"https://api.github.com/repos/dotnet/corefx/issues/2446","number":2446,"state":"open","locked":false,"title":"Performance improvements to Skip and SkipWhile.","user":{"login":"hackcraft","id":1253369,"avatar_url":"https://avatars.githubusercontent.com/u/1253369?v=3","gravatar_id":"","url":"https://api.github.com/users/hackcraft","html_url":"https://github.com/hackcraft","followers_url":"https://api.github.com/users/hackcraft/followers","following_url":"https://api.github.com/users/hackcraft/following{/other_user}","gists_url":"https://api.github.com/users/hackcraft/gists{/gist_id}","starred_url":"https://api.github.com/users/hackcraft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hackcraft/subscriptions","organizations_url":"https://api.github.com/users/hackcraft/orgs","repos_url":"https://api.github.com/users/hackcraft/repos","events_url":"https://api.github.com/users/hackcraft/events{/privacy}","received_events_url":"https://api.github.com/users/hackcraft/received_events","type":"User","site_admin":false},"body":"Fixes #2238\r\n(And I think this is the last bit to separate out of the original PR)\r\n\r\nhttps://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods\r\nshows a comparison of the old and new approaches. Improvements are made in\r\nthe vast majority of cases, and with Skip in particular can be significant.\r\n\r\nEdge-cases of disposal and exception ordering are dealt with; duplicating the\r\nbehaviour found with the current implementation.\r\n\r\n(This ignores the matter of ordered skips as per #2401).","created_at":"2015-07-22T10:43:14Z","updated_at":"2015-07-22T20:38:24Z","closed_at":null,"merged_at":null,"merge_commit_sha":"939f12c8d0dafcdf9a5ff73992bab6bbbd2695e4","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/dotnet/corefx/pulls/2446/commits","review_comments_url":"https://api.github.com/repos/dotnet/corefx/pulls/2446/comments","review_comment_url":"https://api.github.com/repos/dotnet/corefx/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dotnet/corefx/issues/2446/comments","statuses_url":"https://api.github.com/repos/dotnet/corefx/statuses/3658c688bfe585a0fadb14b1b04ecca7c466ab2e","head":{"label":"hackcraft:skip_performance","ref":"skip_performance","sha":"3658c688bfe585a0fadb14b1b04ecca7c466ab2e","user":{"login":"hackcraft","id":1253369,"avatar_url":"https://avatars.githubusercontent.com/u/1253369?v=3","gravatar_id":"","url":"https://api.github.com/users/hackcraft","html_url":"https://github.com/hackcraft","followers_url":"https://api.github.com/users/hackcraft/followers","following_url":"https://api.github.com/users/hackcraft/following{/other_user}","gists_url":"https://api.github.com/users/hackcraft/gists{/gist_id}","starred_url":"https://api.github.com/users/hackcraft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hackcraft/subscriptions","organizations_url":"https://api.github.com/users/hackcraft/orgs","repos_url":"https://api.github.com/users/hackcraft/repos","events_url":"https://api.github.com/users/hackcraft/events{/privacy}","received_events_url":"https://api.github.com/users/hackcraft/received_events","type":"User","site_admin":false},"repo":{"id":38018243,"name":"corefx","full_name":"hackcraft/corefx","owner":{"login":"hackcraft","id":1253369,"avatar_url":"https://avatars.githubusercontent.com/u/1253369?v=3","gravatar_id":"","url":"https://api.github.com/users/hackcraft","html_url":"https://github.com/hackcraft","followers_url":"https://api.github.com/users/hackcraft/followers","following_url":"https://api.github.com/users/hackcraft/following{/other_user}","gists_url":"https://api.github.com/users/hackcraft/gists{/gist_id}","starred_url":"https://api.github.com/users/hackcraft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hackcraft/subscriptions","organizations_url":"https://api.github.com/users/hackcraft/orgs","repos_url":"https://api.github.com/users/hackcraft/repos","events_url":"https://api.github.com/users/hackcraft/events{/privacy}","received_events_url":"https://api.github.com/users/hackcraft/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/hackcraft/corefx","description":"This repo contains the .NET Core foundational libraries, called CoreFX. It includes classes for collections, file systems, console, XML, async and many others.","fork":true,"url":"https://api.github.com/repos/hackcraft/corefx","forks_url":"https://api.github.com/repos/hackcraft/corefx/forks","keys_url":"https://api.github.com/repos/hackcraft/corefx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/hackcraft/corefx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/hackcraft/corefx/teams","hooks_url":"https://api.github.com/repos/hackcraft/corefx/hooks","issue_events_url":"https://api.github.com/repos/hackcraft/corefx/issues/events{/number}","events_url":"https://api.github.com/repos/hackcraft/corefx/events","assignees_url":"https://api.github.com/repos/hackcraft/corefx/assignees{/user}","branches_url":"https://api.github.com/repos/hackcraft/corefx/branches{/branch}","tags_url":"https://api.github.com/repos/hackcraft/corefx/tags","blobs_url":"https://api.github.com/repos/hackcraft/corefx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/hackcraft/corefx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/hackcraft/corefx/git/refs{/sha}","trees_url":"https://api.github.com/repos/hackcraft/corefx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/hackcraft/corefx/statuses/{sha}","languages_url":"https://api.github.com/repos/hackcraft/corefx/languages","stargazers_url":"https://api.github.com/repos/hackcraft/corefx/stargazers","contributors_url":"https://api.github.com/repos/hackcraft/corefx/contributors","subscribers_url":"https://api.github.com/repos/hackcraft/corefx/subscribers","subscription_url":"https://api.github.com/repos/hackcraft/corefx/subscription","commits_url":"https://api.github.com/repos/hackcraft/corefx/commits{/sha}","git_commits_url":"https://api.github.com/repos/hackcraft/corefx/git/commits{/sha}","comments_url":"https://api.github.com/repos/hackcraft/corefx/comments{/number}","issue_comment_url":"https://api.github.com/repos/hackcraft/corefx/issues/comments{/number}","contents_url":"https://api.github.com/repos/hackcraft/corefx/contents/{+path}","compare_url":"https://api.github.com/repos/hackcraft/corefx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/hackcraft/corefx/merges","archive_url":"https://api.github.com/repos/hackcraft/corefx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/hackcraft/corefx/downloads","issues_url":"https://api.github.com/repos/hackcraft/corefx/issues{/number}","pulls_url":"https://api.github.com/repos/hackcraft/corefx/pulls{/number}","milestones_url":"https://api.github.com/repos/hackcraft/corefx/milestones{/number}","notifications_url":"https://api.github.com/repos/hackcraft/corefx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/hackcraft/corefx/labels{/name}","releases_url":"https://api.github.com/repos/hackcraft/corefx/releases{/id}","created_at":"2015-06-25T00:19:13Z","updated_at":"2015-06-25T00:19:32Z","pushed_at":"2015-07-22T10:49:16Z","git_url":"git://github.com/hackcraft/corefx.git","ssh_url":"git@github.com:hackcraft/corefx.git","clone_url":"https://github.com/hackcraft/corefx.git","svn_url":"https://github.com/hackcraft/corefx","homepage":"http://dotnet.github.io/core/","size":25197,"stargazers_count":0,"watchers_count":0,"language":"C#","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"dotnet:master","ref":"master","sha":"244f9b0bc2b34f5d8f2467002b273821e2b99349","user":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"repo":{"id":26295345,"name":"corefx","full_name":"dotnet/corefx","owner":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/dotnet/corefx","description":"This repo contains the .NET Core foundational libraries, called CoreFX. It includes classes for collections, file systems, console, XML, async and many others.","fork":false,"url":"https://api.github.com/repos/dotnet/corefx","forks_url":"https://api.github.com/repos/dotnet/corefx/forks","keys_url":"https://api.github.com/repos/dotnet/corefx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dotnet/corefx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dotnet/corefx/teams","hooks_url":"https://api.github.com/repos/dotnet/corefx/hooks","issue_events_url":"https://api.github.com/repos/dotnet/corefx/issues/events{/number}","events_url":"https://api.github.com/repos/dotnet/corefx/events","assignees_url":"https://api.github.com/repos/dotnet/corefx/assignees{/user}","branches_url":"https://api.github.com/repos/dotnet/corefx/branches{/branch}","tags_url":"https://api.github.com/repos/dotnet/corefx/tags","blobs_url":"https://api.github.com/repos/dotnet/corefx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dotnet/corefx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dotnet/corefx/git/refs{/sha}","trees_url":"https://api.github.com/repos/dotnet/corefx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dotnet/corefx/statuses/{sha}","languages_url":"https://api.github.com/repos/dotnet/corefx/languages","stargazers_url":"https://api.github.com/repos/dotnet/corefx/stargazers","contributors_url":"https://api.github.com/repos/dotnet/corefx/contributors","subscribers_url":"https://api.github.com/repos/dotnet/corefx/subscribers","subscription_url":"https://api.github.com/repos/dotnet/corefx/subscription","commits_url":"https://api.github.com/repos/dotnet/corefx/commits{/sha}","git_commits_url":"https://api.github.com/repos/dotnet/corefx/git/commits{/sha}","comments_url":"https://api.github.com/repos/dotnet/corefx/comments{/number}","issue_comment_url":"https://api.github.com/repos/dotnet/corefx/issues/comments{/number}","contents_url":"https://api.github.com/repos/dotnet/corefx/contents/{+path}","compare_url":"https://api.github.com/repos/dotnet/corefx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dotnet/corefx/merges","archive_url":"https://api.github.com/repos/dotnet/corefx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dotnet/corefx/downloads","issues_url":"https://api.github.com/repos/dotnet/corefx/issues{/number}","pulls_url":"https://api.github.com/repos/dotnet/corefx/pulls{/number}","milestones_url":"https://api.github.com/repos/dotnet/corefx/milestones{/number}","notifications_url":"https://api.github.com/repos/dotnet/corefx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dotnet/corefx/labels{/name}","releases_url":"https://api.github.com/repos/dotnet/corefx/releases{/id}","created_at":"2014-11-06T23:42:48Z","updated_at":"2015-07-22T20:03:46Z","pushed_at":"2015-07-22T19:55:55Z","git_url":"git://github.com/dotnet/corefx.git","ssh_url":"git@github.com:dotnet/corefx.git","clone_url":"https://github.com/dotnet/corefx.git","svn_url":"https://github.com/dotnet/corefx","homepage":"http://dotnet.github.io/core/","size":54785,"stargazers_count":8095,"watchers_count":8095,"language":"C#","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1687,"mirror_url":null,"open_issues_count":472,"forks":1687,"open_issues":472,"watchers":8095,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/2446"},"html":{"href":"https://github.com/dotnet/corefx/pull/2446"},"issue":{"href":"https://api.github.com/repos/dotnet/corefx/issues/2446"},"comments":{"href":"https://api.github.com/repos/dotnet/corefx/issues/2446/comments"},"review_comments":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/2446/comments"},"review_comment":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/2446/commits"},"statuses":{"href":"https://api.github.com/repos/dotnet/corefx/statuses/3658c688bfe585a0fadb14b1b04ecca7c466ab2e"}}}}