{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/roslyn/issues/16795","repository_url":"https://api.github.com/repos/dotnet/roslyn","labels_url":"https://api.github.com/repos/dotnet/roslyn/issues/16795/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/roslyn/issues/16795/comments","events_url":"https://api.github.com/repos/dotnet/roslyn/issues/16795/events","html_url":"https://github.com/dotnet/roslyn/pull/16795","id":203706075,"number":16795,"title":"Extract bind methods for ref type and ref expression","user":{"login":"jcouv","id":12466233,"avatar_url":"https://avatars.githubusercontent.com/u/12466233?v=3","gravatar_id":"","url":"https://api.github.com/users/jcouv","html_url":"https://github.com/jcouv","followers_url":"https://api.github.com/users/jcouv/followers","following_url":"https://api.github.com/users/jcouv/following{/other_user}","gists_url":"https://api.github.com/users/jcouv/gists{/gist_id}","starred_url":"https://api.github.com/users/jcouv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jcouv/subscriptions","organizations_url":"https://api.github.com/users/jcouv/orgs","repos_url":"https://api.github.com/users/jcouv/repos","events_url":"https://api.github.com/users/jcouv/events{/privacy}","received_events_url":"https://api.github.com/users/jcouv/received_events","type":"User","site_admin":false},"labels":[{"id":166673288,"url":"https://api.github.com/repos/dotnet/roslyn/labels/Area-Compilers","name":"Area-Compilers","color":"5319e7","default":false},{"id":197452922,"url":"https://api.github.com/repos/dotnet/roslyn/labels/cla-already-signed","name":"cla-already-signed","color":"009800","default":false}],"state":"open","locked":false,"assignee":{"login":"jcouv","id":12466233,"avatar_url":"https://avatars.githubusercontent.com/u/12466233?v=3","gravatar_id":"","url":"https://api.github.com/users/jcouv","html_url":"https://github.com/jcouv","followers_url":"https://api.github.com/users/jcouv/followers","following_url":"https://api.github.com/users/jcouv/following{/other_user}","gists_url":"https://api.github.com/users/jcouv/gists{/gist_id}","starred_url":"https://api.github.com/users/jcouv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jcouv/subscriptions","organizations_url":"https://api.github.com/users/jcouv/orgs","repos_url":"https://api.github.com/users/jcouv/repos","events_url":"https://api.github.com/users/jcouv/events{/privacy}","received_events_url":"https://api.github.com/users/jcouv/received_events","type":"User","site_admin":false},"assignees":[{"login":"jcouv","id":12466233,"avatar_url":"https://avatars.githubusercontent.com/u/12466233?v=3","gravatar_id":"","url":"https://api.github.com/users/jcouv","html_url":"https://github.com/jcouv","followers_url":"https://api.github.com/users/jcouv/followers","following_url":"https://api.github.com/users/jcouv/following{/other_user}","gists_url":"https://api.github.com/users/jcouv/gists{/gist_id}","starred_url":"https://api.github.com/users/jcouv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jcouv/subscriptions","organizations_url":"https://api.github.com/users/jcouv/orgs","repos_url":"https://api.github.com/users/jcouv/repos","events_url":"https://api.github.com/users/jcouv/events{/privacy}","received_events_url":"https://api.github.com/users/jcouv/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/dotnet/roslyn/milestones/16","html_url":"https://github.com/dotnet/roslyn/milestone/16","labels_url":"https://api.github.com/repos/dotnet/roslyn/milestones/16/labels","id":1571384,"number":16,"title":"2.0 (RTM)","description":"","creator":{"login":"davkean","id":1103906,"avatar_url":"https://avatars.githubusercontent.com/u/1103906?v=3","gravatar_id":"","url":"https://api.github.com/users/davkean","html_url":"https://github.com/davkean","followers_url":"https://api.github.com/users/davkean/followers","following_url":"https://api.github.com/users/davkean/following{/other_user}","gists_url":"https://api.github.com/users/davkean/gists{/gist_id}","starred_url":"https://api.github.com/users/davkean/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davkean/subscriptions","organizations_url":"https://api.github.com/users/davkean/orgs","repos_url":"https://api.github.com/users/davkean/repos","events_url":"https://api.github.com/users/davkean/events{/privacy}","received_events_url":"https://api.github.com/users/davkean/received_events","type":"User","site_admin":false},"open_issues":82,"closed_issues":398,"state":"open","created_at":"2016-02-08T21:54:04Z","updated_at":"2017-01-27T18:29:14Z","due_on":null,"closed_at":null},"comments":0,"created_at":"2017-01-27T18:28:45Z","updated_at":"2017-01-27T18:29:30Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/dotnet/roslyn/pulls/16795","html_url":"https://github.com/dotnet/roslyn/pull/16795","diff_url":"https://github.com/dotnet/roslyn/pull/16795.diff","patch_url":"https://github.com/dotnet/roslyn/pull/16795.patch"},"body":"**Customer scenario**\r\nThe customer uses a long/fluent invocation, such as `new C().M().M(). ... .M();`. That invocation would compile for very long expressions (2550 iterations on my box) with version 1.3.2 of the compiler. But such invocation only compiles for 630 iterations with dev15 RC3.\r\nWith this fix, it is possible to compile 1690 iterations (a 30% reduction from 1.3.2).\r\n\r\n**Bugs this fixes:** \r\nMitigates https://github.com/dotnet/roslyn/issues/16669\r\n\r\n**Workarounds, if any**\r\nUpdate your code and chop of the fluent expression into chunks.\r\n\r\n**Root cause analysis:**\r\nThere are three factors which caused this:\r\n1. The size of a stack frame for some binding methods grew. The largest one is `BindExpressionInternal` which grew from 72 bytes (in 1.3.2) to 272 bytes (in RC.3). It is called recursively when binding fluent expressions.\r\n2. The binding now involves an `ExpressionVariableFinder`, which was introduced to detect locals declared by expressions. This requires a walk through the syntax of the fluent expression and is currently implemented as a recursion.\r\n3. When the `ExpressionVariableFinder` does this recursive walk, it uses a BCL API to check if it is getting close to a stack overflow. Unfortunately, this API is trigger happy. Although the default stack size is 1MB, this API gives up at 500kB+1.\r\n\r\nThe fix in this PR involves two parts:\r\n1. We reduce problem (1) above, by extracting two blocks of code from `BindExpressionInternal` to new methods. This reduces the size of the stack frame from 272 back to 116. This is still up from 72 (in 1.3.2).\r\n2. We removing problem (3) above, by modifying the `ExpressionVariableFinder` to walk the syntax without a stack guard. This allows the full 1MB of stack space to be used.\r\n\r\nNote that problem (1) is largely but not completely mitigated and problem (2) remains. This results in a 30% overall decrease in expressions we can normally. \r\nFurther optimizations can be done past RTW:\r\n- we may be able to further reduce the frame size for some methods,\r\n- we could improve the stack guard to be less trigger happy,\r\n- when we get close to the stack size limit, we could pause the thread and continue the recursive computation on a fresh thread (starting from a shallow stack again),\r\n- modify some of our node walking to avoid recursion.\r\n\r\n**Risk**\r\n**Performance impact**\r\nLow. See description of the fix above.\r\n\r\n**Is this a regression from a previous update?**\r\nYes, this is a progressive regression from 1.3.2.\r\nOn \"1.3.2\" the max is around 2550.\r\nOn \"2.0.0-beta1\" the max is around 900.\r\nOn \"2.0.0-beta5\" the max is around 670.\r\nOn “2.0.0-rc” the max is around 670.\r\nOn “2.0.0-rc2” the max is around 630.\r\n\r\nWe're adding a test for such long expressions, so that we catch such regressions in the future. The difficulty is that the stack and thread size is platform and environment dependent, so we're still looking for a good way to control that in our automation.\r\n\r\n**How was the bug found?**\r\nWe had multiple customer reports that we categorized as \"by design\". The user should not write such massive expressions. But apparently a number of customers do and I started to suspect the compiler may have become more sensitive to stack overflows. Ad hoc testing confirmed this."},"comment":{"url":"https://api.github.com/repos/dotnet/roslyn/issues/comments/275737877","html_url":"https://github.com/dotnet/roslyn/pull/16795#issuecomment-275737877","issue_url":"https://api.github.com/repos/dotnet/roslyn/issues/16795","id":275737877,"user":{"login":"jcouv","id":12466233,"avatar_url":"https://avatars.githubusercontent.com/u/12466233?v=3","gravatar_id":"","url":"https://api.github.com/users/jcouv","html_url":"https://github.com/jcouv","followers_url":"https://api.github.com/users/jcouv/followers","following_url":"https://api.github.com/users/jcouv/following{/other_user}","gists_url":"https://api.github.com/users/jcouv/gists{/gist_id}","starred_url":"https://api.github.com/users/jcouv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jcouv/subscriptions","organizations_url":"https://api.github.com/users/jcouv/orgs","repos_url":"https://api.github.com/users/jcouv/repos","events_url":"https://api.github.com/users/jcouv/events{/privacy}","received_events_url":"https://api.github.com/users/jcouv/received_events","type":"User","site_admin":false},"created_at":"2017-01-27T18:29:30Z","updated_at":"2017-01-27T18:29:30Z","body":"@dotnet/roslyn-compiler for review."}}
{"actor":{"display_login":"jcouv"}}