{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4265","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4265/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4265/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4265/events","html_url":"https://github.com/RobotLocomotion/drake/pull/4265","id":191887811,"number":4265,"title":"guard against dynamic allocation in main acrobot_plant dynamics and output loops.","user":{"login":"RussTedrake","id":6442292,"avatar_url":"https://avatars.githubusercontent.com/u/6442292?v=3","gravatar_id":"","url":"https://api.github.com/users/RussTedrake","html_url":"https://github.com/RussTedrake","followers_url":"https://api.github.com/users/RussTedrake/followers","following_url":"https://api.github.com/users/RussTedrake/following{/other_user}","gists_url":"https://api.github.com/users/RussTedrake/gists{/gist_id}","starred_url":"https://api.github.com/users/RussTedrake/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RussTedrake/subscriptions","organizations_url":"https://api.github.com/users/RussTedrake/orgs","repos_url":"https://api.github.com/users/RussTedrake/repos","events_url":"https://api.github.com/users/RussTedrake/events{/privacy}","received_events_url":"https://api.github.com/users/RussTedrake/received_events","type":"User","site_admin":false},"labels":[{"id":366762542,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/priority:%20medium","name":"priority: medium","color":"e99695","default":false},{"id":366761608,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/team:%20software%20core","name":"team: software core","color":"0052cc","default":false},{"id":77797802,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/type:%20cleanup","name":"type: cleanup","color":"009800","default":false}],"state":"open","locked":false,"assignee":{"login":"sherm1","id":4088016,"avatar_url":"https://avatars.githubusercontent.com/u/4088016?v=3","gravatar_id":"","url":"https://api.github.com/users/sherm1","html_url":"https://github.com/sherm1","followers_url":"https://api.github.com/users/sherm1/followers","following_url":"https://api.github.com/users/sherm1/following{/other_user}","gists_url":"https://api.github.com/users/sherm1/gists{/gist_id}","starred_url":"https://api.github.com/users/sherm1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sherm1/subscriptions","organizations_url":"https://api.github.com/users/sherm1/orgs","repos_url":"https://api.github.com/users/sherm1/repos","events_url":"https://api.github.com/users/sherm1/events{/privacy}","received_events_url":"https://api.github.com/users/sherm1/received_events","type":"User","site_admin":false},"assignees":[{"login":"sherm1","id":4088016,"avatar_url":"https://avatars.githubusercontent.com/u/4088016?v=3","gravatar_id":"","url":"https://api.github.com/users/sherm1","html_url":"https://github.com/sherm1","followers_url":"https://api.github.com/users/sherm1/followers","following_url":"https://api.github.com/users/sherm1/following{/other_user}","gists_url":"https://api.github.com/users/sherm1/gists{/gist_id}","starred_url":"https://api.github.com/users/sherm1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sherm1/subscriptions","organizations_url":"https://api.github.com/users/sherm1/orgs","repos_url":"https://api.github.com/users/sherm1/repos","events_url":"https://api.github.com/users/sherm1/events{/privacy}","received_events_url":"https://api.github.com/users/sherm1/received_events","type":"User","site_admin":false},{"login":"david-german-tri","id":17437069,"avatar_url":"https://avatars.githubusercontent.com/u/17437069?v=3","gravatar_id":"","url":"https://api.github.com/users/david-german-tri","html_url":"https://github.com/david-german-tri","followers_url":"https://api.github.com/users/david-german-tri/followers","following_url":"https://api.github.com/users/david-german-tri/following{/other_user}","gists_url":"https://api.github.com/users/david-german-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/david-german-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david-german-tri/subscriptions","organizations_url":"https://api.github.com/users/david-german-tri/orgs","repos_url":"https://api.github.com/users/david-german-tri/repos","events_url":"https://api.github.com/users/david-german-tri/events{/privacy}","received_events_url":"https://api.github.com/users/david-german-tri/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2016-11-27T21:46:57Z","updated_at":"2016-11-28T16:04:54Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/4265","html_url":"https://github.com/RobotLocomotion/drake/pull/4265","diff_url":"https://github.com/RobotLocomotion/drake/pull/4265.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/4265.patch"},"body":"This is an exercise that we should do throughout the code:\r\nhttp://eigen.tuxfamily.org/index.php?title=FAQ#Where_in_my_program_are_temporary_objects_created.3F\r\n\r\nIt was interesting to do it just in the AcrobotPlant code.  In this (non-critical) code, I think it makes sense to leave the guards in.\r\n\r\nN.B. - I think excessive Eigen dynamic allocation is one of the major risks in System2.0 (if we find it painful later, it could force an API change).  This was a baby spike test.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/4265)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/263311589","html_url":"https://github.com/RobotLocomotion/drake/pull/4265#issuecomment-263311589","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4265","id":263311589,"user":{"login":"david-german-tri","id":17437069,"avatar_url":"https://avatars.githubusercontent.com/u/17437069?v=3","gravatar_id":"","url":"https://api.github.com/users/david-german-tri","html_url":"https://github.com/david-german-tri","followers_url":"https://api.github.com/users/david-german-tri/followers","following_url":"https://api.github.com/users/david-german-tri/following{/other_user}","gists_url":"https://api.github.com/users/david-german-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/david-german-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david-german-tri/subscriptions","organizations_url":"https://api.github.com/users/david-german-tri/orgs","repos_url":"https://api.github.com/users/david-german-tri/repos","events_url":"https://api.github.com/users/david-german-tri/events{/privacy}","received_events_url":"https://api.github.com/users/david-german-tri/received_events","type":"User","site_admin":false},"created_at":"2016-11-28T16:04:54Z","updated_at":"2016-11-28T16:04:54Z","body":"\n\n\n\nReview status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.\n\n---\n\n*[a discussion](https://reviewable.io:443/reviews/robotlocomotion/drake/4265#-KXcqNoPxI1eXlsPq2q7:-KXfdOXUAC0VfL85lbAy:bfttn65) (no related file):*\n<details><summary><i>Previously, RussTedrake (Russ Tedrake) wrote…</i></summary><blockquote>\n\nWhen I started, I expect to write something like that (precisely to obtain the push/pop).  But eigen's method was convenient enough to start with.  Agreed that we need better semantics if we're going to do this sort of thing everywhere.  perhaps a todo is sufficient for this first instance to get in?\n</blockquote></details>\n\n-> #4269.  This is a nifty trick for local testing, but given the ABI and thread safety problems, I'm uneasy about merging this PR even with a TODO.\n\n---\n\n*[drake/examples/Acrobot/acrobot_plant.cc, line 1 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/4265#-KXferGe93CGHT2YcrmW:-KXferGfuuCq9ghRixvq:b4j4wr8) ([raw file](https://github.com/robotlocomotion/drake/blob/b1b48a0cec5993320b394fa7a19cf926287b5bfd/drake/examples/Acrobot/acrobot_plant.cc#L1)):*\n> ```cpp\n> #define EIGEN_RUNTIME_NO_MALLOC\n> ```\n\n@jwnimmer-tri points out that this may mess with the ABI of Eigen itself.\n\n---\n\n*[drake/examples/Acrobot/acrobot_plant.cc, line 31 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/4265#-KXff1u8hqVofZMsQgeY:-KXff1u8hqVofZMsQgeZ:b3jrmnz) ([raw file](https://github.com/robotlocomotion/drake/blob/b1b48a0cec5993320b394fa7a19cf926287b5bfd/drake/examples/Acrobot/acrobot_plant.cc#L31)):*\n> ```cpp\n> void AcrobotPlant<T>::EvalOutput(const systems::Context<T>& context,\n>                                  systems::SystemOutput<T>* output) const {\n>   Eigen::internal::set_is_malloc_allowed(!std::is_same<T, double>::value);\n> ```\n\nThis (and below) is not thread-safe; it modifies a static bool.  Even adding a wrapper with a lock and a stack would not make it thread-safe in a practical sense, since another thread could change the behavior mid-Eval.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/4265)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"david-german-tri"}}