{"action":"created","issue":{"url":"https://api.github.com/repos/elastic/elasticsearch/issues/18400","repository_url":"https://api.github.com/repos/elastic/elasticsearch","labels_url":"https://api.github.com/repos/elastic/elasticsearch/issues/18400/labels{/name}","comments_url":"https://api.github.com/repos/elastic/elasticsearch/issues/18400/comments","events_url":"https://api.github.com/repos/elastic/elasticsearch/issues/18400/events","html_url":"https://github.com/elastic/elasticsearch/pull/18400","id":155243592,"number":18400,"title":"painless: use Java 9 Indy String Concats, if available","user":{"login":"uschindler","id":1005388,"avatar_url":"https://avatars.githubusercontent.com/u/1005388?v=3","gravatar_id":"","url":"https://api.github.com/users/uschindler","html_url":"https://github.com/uschindler","followers_url":"https://api.github.com/users/uschindler/followers","following_url":"https://api.github.com/users/uschindler/following{/other_user}","gists_url":"https://api.github.com/users/uschindler/gists{/gist_id}","starred_url":"https://api.github.com/users/uschindler/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uschindler/subscriptions","organizations_url":"https://api.github.com/users/uschindler/orgs","repos_url":"https://api.github.com/users/uschindler/repos","events_url":"https://api.github.com/users/uschindler/events{/privacy}","received_events_url":"https://api.github.com/users/uschindler/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/elastic/elasticsearch/labels/:Plugin%20Lang%20Painless","name":":Plugin Lang Painless","color":"009800"},{"url":"https://api.github.com/repos/elastic/elasticsearch/labels/v5.0.0","name":"v5.0.0","color":"dddddd"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2016-05-17T12:08:59Z","updated_at":"2016-05-17T22:35:36Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/elastic/elasticsearch/pulls/18400","html_url":"https://github.com/elastic/elasticsearch/pull/18400","diff_url":"https://github.com/elastic/elasticsearch/pull/18400.diff","patch_url":"https://github.com/elastic/elasticsearch/pull/18400.patch"},"body":"This PR adds support to painless for indyfied string concats, which is a new feature of Java 9. Instead of creating a StringBuilder in byte code and then appending all parts, the recommended way for bytecode in Java 9 is to use use `invokedynamic` on `java.lang.invoke.StringConcatFactory`. This allows many improvements in the JVM:\r\n\r\n- Hotspot is better in optimizing this, as the code is just a single method call with many (primitive or reference) arguments\r\n- It knows the size of the result array before (e.g. by summing up the lengths of all parts)\r\n- It can directly create a \"compact string\", while `StringBuilder` still uses a `char[]` inbetween\r\n\r\nSee http://openjdk.java.net/jeps/280 and http://cr.openjdk.java.net/~shade/8085796/notes.txt for more details. Not all of the above optimizations are implemented in JDK/Hotspot yet, but the old code will no longer get improvements in JDK, as it is too complicated to analyze and breaks easily.\r\n\r\nThe new code checks if the `StringConcatFactory` is available at run time with correct signature. Otherwise the byte code is generated as before. If indy string concats are enabled, all parts are pushed with their original types to stack and the MethodWriter's concat method just records the types, so it can build the descriptor for the single method call afterwards.\r\n\r\nAs number of arguments to concat is limited to 200, the code calls the invokedyanmic always after 200 items and just leaves the return value on stack as first param for next round. While testing this I found a parser slowness bug in painless (see #18398)."},"comment":{"url":"https://api.github.com/repos/elastic/elasticsearch/issues/comments/219874834","html_url":"https://github.com/elastic/elasticsearch/pull/18400#issuecomment-219874834","issue_url":"https://api.github.com/repos/elastic/elasticsearch/issues/18400","id":219874834,"user":{"login":"uschindler","id":1005388,"avatar_url":"https://avatars.githubusercontent.com/u/1005388?v=3","gravatar_id":"","url":"https://api.github.com/users/uschindler","html_url":"https://github.com/uschindler","followers_url":"https://api.github.com/users/uschindler/followers","following_url":"https://api.github.com/users/uschindler/following{/other_user}","gists_url":"https://api.github.com/users/uschindler/gists{/gist_id}","starred_url":"https://api.github.com/users/uschindler/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uschindler/subscriptions","organizations_url":"https://api.github.com/users/uschindler/orgs","repos_url":"https://api.github.com/users/uschindler/repos","events_url":"https://api.github.com/users/uschindler/events{/privacy}","received_events_url":"https://api.github.com/users/uschindler/received_events","type":"User","site_admin":false},"created_at":"2016-05-17T22:35:36Z","updated_at":"2016-05-17T22:35:36Z","body":"> As far as compile option, do you have concerns about correctness? From my perspective it is a task for jenkins and we don't need it.\r\n\r\nNo concerns. I just say: Java 9 is not yet finalized. The curennt code only activates concats if the method signature matches 100%, otherwise it uses string buffer, so risk is low. On Java 8 it will never be used, as StringConcatFactory does not exist there.\r\n\r\nThe configuration option was just meant as a way to disable it, if it really breaks with a later JDK9 EA release (e.g. if then change the magic number 200 shortly before release to say 150...). Currently its documented to be 200 in Javadocs, but those are not yet mandatory. I will suggest on the Mailing list to have a integer constant with that number in the StringConcatFactory."}}