The fix is in JDK 9 now. I will propose this to be backported this to 8u, which
means it should appear in some 8u release soon, but I can’t make any promise
about a specific release or date.
Hannes
> Am 09.01.2017 um 23:44 schrieb Jesus Luzon <jlu...@riotgames.com>:
>
> Hey folks,
>
> Looked at the ticket and noticed it has been marked as fixed. woot! Thank you
> so much!
>
> My team has been asking me if I can find out the date this fix will get
> released. I noticed the version on there (version 9) states it has no release
> date yet. Is there any tentative date you all have for the net release this
> would be included in? Even a ballpark figure would be awesome.
>
> Thanks.
>
> On Fri, Dec 9, 2016 at 2:49 AM, Jesus Luzon <jlu...@riotgames.com> wrote:
> Wow I totally missed the var. Thanks for that! I actually had changed our
> code to use ThreadLocal ScriptObjectMirrors created from the CompiledScript
> with the following code:
>
> ScriptObjectMirror mirror = mirrorThreadLocal.get();
> if (mirror == null) {
> Bindings bindings = compiledScript.getEngine().createBindings();
> compiledScript.eval(bindings);
> mirror = (ScriptObjectMirror) bindings.get("transform");
> mirrorThreadLocal.set(mirror);
> }
> return (String) mirror.call(null, something);
>
> Given that we will always run standalone functions to filter data, is there
> one we should be using over the other?
>
> Lastly thanks for filing the bug. I'll keep track of it so I can make sure to
> upgrade when a fix comes out.
>
> On Fri, Dec 9, 2016 at 2:37 AM, Hannes Wallnöfer
> <hannes.wallnoe...@oracle.com> wrote:
> Jesus,
>
> I looked at this issue more in depth and there are two problems here, one on
> your side and one on ours:
>
> The problem with your code is that you left out the „var“ declaration of the
> „response“ variable in the third line of your function. This means that
> „response“ is allocated as a global variable outside the scope of the
> function. When you run this in parallel with multiple threads you actually
> access the same object from multiple threads, meaning your output will be
> corrupt.
>
> Because of things like this, it is actually a good idea to run JavaScript in
> strict mode (e.g. by using „use strict“ directive in your JavaScript code, or
> by creating the engine with -strict option). Another solution would be to use
> multiple engines or bindings, which should not have a big performance impact
> once the other problem is fixed.
>
> Which leads us to the problem on our side, which is performance of sparse
> arrays. As you have noticed, using something that is not a valid array index
> greatly improves performance, and that is because some array element
> assignment are implemented very inefficiently in Nashorn. I’ve filed a bug
> for this:
>
> https://bugs.openjdk.java.net/browse/JDK-8170977
>
> Once this bug is fixed, using numeric keys should be roughly as fast as using
> non-numeric ones.
>
> Hannes
>
>
> > Am 09.12.2016 um 04:47 schrieb Jesus Luzon <jlu...@riotgames.com>:
> >
> > I came up with a workaround which is getting us through this but it
> > definitely seems like this is a bug. The gist is I append an arbitrary
> > string (we made it complex on our end to make it unlikely something else
> > has this string inside) to the key and then remove all instances of said
> > string on the JSON String resulting from calling stringify. We are leaving
> > this for overnight testing but it showed promising results immediately.
> >
> > String script = "function transform(input) {" +
> > "var result = JSON.parse(input);" +
> > "response = {};\n" +
> > "for (var i = 0; i < result.length; i++) {\n" +
> > " var summoner = {};\n" +
> > " summoner.id = result[i].id;\n" +
> > " summoner.name = result[i].name;\n" +
> > " summoner.profileIconId = result[i].profileIconId;\n" +
> > " summoner.revisionDate = result[i].revisionDate;\n" +
> > " summoner.summonerLevel = result[i].level;\n" +
> > " response[\"someArbitraryString\" + summoner.id] =
> > summoner;\n" +
> > "}\n" +
> > "var stringy = JSON.stringify(response);" +
> > "result = stringy.replace(/someArbitraryString/g, \"\");" +
> > "return result;" +
> > "};";
> >
> > On Thu, Dec 8, 2016 at 4:54 PM, Jesus Luzon <jlu...@riotgames.com> wrote:
> > It doesn't seem like changing to a string fixed it. We dug deeper and found
> > a very interesting issue though. After running the invocable a few thousand
> > times, when we put a breakpoint in ArrayData.computerIteratorKeys() and we
> > found that it would stop at an execution from SparseArrayData with an
> > underlying array of size 923392, stored length of 461672 (this is one of
> > our IDs, the largest 6 digit one). Because of this we tried a run changing
> > all the IDs in the array we pass in to number from 1-38 and this thing went
> > blazing fast. When putting a break point in the same place, this array in
> > SpareArrayData was now of size 39. We then changed an id in the array we
> > take in to be 1337 and the size of the array in the SparseArrayData was
> > 1338.
> >
> > I don't understand why or how to prevent but it's using this ID as an index
> > for SpareArrayData underlying array. If someone can help me find a
> > workaround for this I would be extremely grateful.
> >
> > On Thu, Dec 8, 2016 at 3:52 AM, Hannes Wallnöfer
> > <hannes.wallnoe...@oracle.com> wrote:
> > Yes, this is very likely to be the cause of the problem. However, I do
> > think we should be able to handle sparse array data better, so it’s quite
> > possible that you exposed some weak spot in our implementation. I’ll have a
> > look into what’s going on.
> >
> > Hannes
> >
> >
> > > Am 08.12.2016 um 00:40 schrieb Jesus Luzon <jlu...@riotgames.com>:
> > >
> > > Looks like the memory leak is due to the way we wrote our code and how
> > > javascript works. I was expecting the line response[summoner.id] =
> > > summoner; to build a map but it turns out that if you use a number as the
> > > "key", javscript automatically fills the indexes in the middle with null
> > > (Undefined type?). When these IDs are very large, it is creating huge
> > > arrays that take longer to garbage collect than the code executing. I am
> > > about to start testing this on our end to make sure we see the
> > > improvements we expect.
> > >
> > > Does this idea seem like it is reasonably possible?
> > >
> > > On Wed, Dec 7, 2016 at 11:23 AM, Jesus Luzon <jlu...@riotgames.com> wrote:
> > > Yes, it's an array of objects which I'll paste. And yes, I'm just calling
> > > invokeFunction from many many different threads. I'm also going to go
> > > back and take a look at all the heap dumps we have to re-confirm what I
> > > mentioned.
> > >
> > > "[\n" +
> > > " {\n" +
> > > " \"id\": 6011511,\n" +
> > > " \"accountId\": 203192481,\n" +
> > > " \"name\": \"Adam Pro Qardaş\",\n" +
> > > " \"profileIconId\": 25,\n" +
> > > " \"level\": 5,\n" +
> > > " \"expPoints\": 83,\n" +
> > > " \"infPoints\": 1475,\n" +
> > > " \"revisionDate\": 1406631727000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 2810674,\n" +
> > > " \"accountId\": 200706913,\n" +
> > > " \"name\": \"ABZ Devrim\",\n" +
> > > " \"profileIconId\": 663,\n" +
> > > " \"level\": 13,\n" +
> > > " \"expPoints\": 982,\n" +
> > > " \"infPoints\": 10472,\n" +
> > > " \"revisionDate\": 1450791227000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5411195,\n" +
> > > " \"accountId\": 202647689,\n" +
> > > " \"name\": \"Ace HypcronN\",\n" +
> > > " \"profileIconId\": 911,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 73,\n" +
> > > " \"infPoints\": 182445,\n" +
> > > " \"revisionDate\": 1480781650000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 1363020,\n" +
> > > " \"accountId\": 1357837,\n" +
> > > " \"name\": \"AdanaLee\",\n" +
> > > " \"profileIconId\": 502,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 125,\n" +
> > > " \"infPoints\": 719299,\n" +
> > > " \"revisionDate\": 1480530778000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 8261198,\n" +
> > > " \"accountId\": 205027096,\n" +
> > > " \"name\": \"Achilehuz\",\n" +
> > > " \"profileIconId\": 1381,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 10,\n" +
> > > " \"infPoints\": 158603,\n" +
> > > " \"revisionDate\": 1480770307000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 12685857,\n" +
> > > " \"accountId\": 207591166,\n" +
> > > " \"name\": \"acımasızpicc\",\n" +
> > > " \"profileIconId\": 9,\n" +
> > > " \"level\": 21,\n" +
> > > " \"expPoints\": 840,\n" +
> > > " \"infPoints\": 16659,\n" +
> > > " \"revisionDate\": 1480515325000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 10860127,\n" +
> > > " \"accountId\": 206507727,\n" +
> > > " \"name\": \"AAngelFlyy\",\n" +
> > > " \"profileIconId\": 1395,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 10,\n" +
> > > " \"infPoints\": 73111,\n" +
> > > " \"revisionDate\": 1480787870000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 3292376,\n" +
> > > " \"accountId\": 201048714,\n" +
> > > " \"name\": \"ACAB1907\",\n" +
> > > " \"profileIconId\": 20,\n" +
> > > " \"level\": 6,\n" +
> > > " \"expPoints\": 305,\n" +
> > > " \"infPoints\": 2107,\n" +
> > > " \"revisionDate\": 1402448089000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 461671,\n" +
> > > " \"accountId\": 446571,\n" +
> > > " \"name\": \"Acta Est Fabulâ\",\n" +
> > > " \"profileIconId\": 1435,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 47,\n" +
> > > " \"infPoints\": 644672,\n" +
> > > " \"revisionDate\": 1480626505000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 394183,\n" +
> > > " \"accountId\": 379083,\n" +
> > > " \"name\": \"acekse4\",\n" +
> > > " \"profileIconId\": 27,\n" +
> > > " \"level\": 5,\n" +
> > > " \"expPoints\": 223,\n" +
> > > " \"infPoints\": 908,\n" +
> > > " \"revisionDate\": 1348116544000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5941247,\n" +
> > > " \"accountId\": 203106300,\n" +
> > > " \"name\": \"abdul7878\",\n" +
> > > " \"profileIconId\": 26,\n" +
> > > " \"level\": 3,\n" +
> > > " \"expPoints\": 10,\n" +
> > > " \"infPoints\": 401,\n" +
> > > " \"revisionDate\": 1406029148000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 2467446,\n" +
> > > " \"accountId\": 200459837,\n" +
> > > " \"name\": \"ActionC\",\n" +
> > > " \"profileIconId\": 986,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 74,\n" +
> > > " \"infPoints\": 401367,\n" +
> > > " \"revisionDate\": 1480808608000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 9402979,\n" +
> > > " \"accountId\": 205698832,\n" +
> > > " \"name\": \"Ablenia \",\n" +
> > > " \"profileIconId\": 1129,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 19,\n" +
> > > " \"infPoints\": 163518,\n" +
> > > " \"revisionDate\": 1480687603000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 13187505,\n" +
> > > " \"accountId\": 207898213,\n" +
> > > " \"name\": \"aDaMiYiM\",\n" +
> > > " \"profileIconId\": 1301,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 116,\n" +
> > > " \"infPoints\": 45214,\n" +
> > > " \"revisionDate\": 1480793258000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 4141059,\n" +
> > > " \"accountId\": 201688290,\n" +
> > > " \"name\": \"AbiminÇarı\",\n" +
> > > " \"profileIconId\": 898,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 152,\n" +
> > > " \"infPoints\": 752477,\n" +
> > > " \"revisionDate\": 1480635961000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5702134,\n" +
> > > " \"accountId\": 202899395,\n" +
> > > " \"name\": \"Above the Clouds\",\n" +
> > > " \"profileIconId\": 684,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 110,\n" +
> > > " \"infPoints\": 288096,\n" +
> > > " \"revisionDate\": 1471011372000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5810740,\n" +
> > > " \"accountId\": 202985228,\n" +
> > > " \"name\": \"aBimm\",\n" +
> > > " \"profileIconId\": 11,\n" +
> > > " \"level\": 13,\n" +
> > > " \"expPoints\": 1180,\n" +
> > > " \"infPoints\": 10736,\n" +
> > > " \"revisionDate\": 1409832684000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5817751,\n" +
> > > " \"accountId\": 203050678,\n" +
> > > " \"name\": \"AD Glorıam\",\n" +
> > > " \"profileIconId\": 982,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 111,\n" +
> > > " \"infPoints\": 304658,\n" +
> > > " \"revisionDate\": 1480795250000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 9851802,\n" +
> > > " \"accountId\": 206011054,\n" +
> > > " \"name\": \"AdarAllame\",\n" +
> > > " \"profileIconId\": 911,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 48,\n" +
> > > " \"infPoints\": 73763,\n" +
> > > " \"revisionDate\": 1479422812000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 12735622,\n" +
> > > " \"accountId\": 207587019,\n" +
> > > " \"name\": \"absinthe666\",\n" +
> > > " \"profileIconId\": 903,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 83,\n" +
> > > " \"infPoints\": 40302,\n" +
> > > " \"revisionDate\": 1480782923000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 6371389,\n" +
> > > " \"accountId\": 203416952,\n" +
> > > " \"name\": \"adamsatıcı\",\n" +
> > > " \"profileIconId\": 3,\n" +
> > > " \"level\": 4,\n" +
> > > " \"expPoints\": 17,\n" +
> > > " \"infPoints\": 685,\n" +
> > > " \"revisionDate\": 1409320171000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 7828139,\n" +
> > > " \"accountId\": 204927980,\n" +
> > > " \"name\": \"AbsoluteForce\",\n" +
> > > " \"profileIconId\": 950,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 139,\n" +
> > > " \"infPoints\": 208789,\n" +
> > > " \"revisionDate\": 1480804396000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 1373229,\n" +
> > > " \"accountId\": 1358441,\n" +
> > > " \"name\": \"AbsoluteDeath\",\n" +
> > > " \"profileIconId\": 7,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 34,\n" +
> > > " \"infPoints\": 223655,\n" +
> > > " \"revisionDate\": 1471867646000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 7694972,\n" +
> > > " \"accountId\": 204803668,\n" +
> > > " \"name\": \"ac pnp\",\n" +
> > > " \"profileIconId\": 937,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 161,\n" +
> > > " \"infPoints\": 249681,\n" +
> > > " \"revisionDate\": 1480801507000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 1373524,\n" +
> > > " \"accountId\": 1350474,\n" +
> > > " \"name\": \"Abdüü\",\n" +
> > > " \"profileIconId\": 1301,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 103,\n" +
> > > " \"infPoints\": 286803,\n" +
> > > " \"revisionDate\": 1476621827000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 1650227,\n" +
> > > " \"accountId\": 200000503,\n" +
> > > " \"name\": \"AD Ambrosia\",\n" +
> > > " \"profileIconId\": 1152,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 139,\n" +
> > > " \"infPoints\": 156333,\n" +
> > > " \"revisionDate\": 1480805320000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 8331358,\n" +
> > > " \"accountId\": 205073925,\n" +
> > > " \"name\": \"acarmanyust2\",\n" +
> > > " \"profileIconId\": 0,\n" +
> > > " \"level\": 2,\n" +
> > > " \"expPoints\": 43,\n" +
> > > " \"infPoints\": 318,\n" +
> > > " \"revisionDate\": 1423915139000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 1862106,\n" +
> > > " \"accountId\": 200139838,\n" +
> > > " \"name\": \"aboU\",\n" +
> > > " \"profileIconId\": 1155,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 0,\n" +
> > > " \"infPoints\": 412616,\n" +
> > > " \"revisionDate\": 1480771055000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 2362628,\n" +
> > > " \"accountId\": 685649,\n" +
> > > " \"name\": \"AcıFısTık\",\n" +
> > > " \"profileIconId\": 1074,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 48,\n" +
> > > " \"infPoints\": 233882,\n" +
> > > " \"revisionDate\": 1480786233000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 4323909,\n" +
> > > " \"accountId\": 201917672,\n" +
> > > " \"name\": \"Addrenalin\",\n" +
> > > " \"profileIconId\": 603,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 55,\n" +
> > > " \"infPoints\": 220605,\n" +
> > > " \"revisionDate\": 1432647338000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 377206,\n" +
> > > " \"accountId\": 362106,\n" +
> > > " \"name\": \"Aburame Shino\",\n" +
> > > " \"profileIconId\": 844,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 84,\n" +
> > > " \"infPoints\": 354087,\n" +
> > > " \"revisionDate\": 1477666556000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5377433,\n" +
> > > " \"accountId\": 202697921,\n" +
> > > " \"name\": \"AcEcolton35\",\n" +
> > > " \"profileIconId\": 984,\n" +
> > > " \"level\": 25,\n" +
> > > " \"expPoints\": 751,\n" +
> > > " \"infPoints\": 30061,\n" +
> > > " \"revisionDate\": 1475503024000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 2381404,\n" +
> > > " \"accountId\": 200333680,\n" +
> > > " \"name\": \"adafakaaa\",\n" +
> > > " \"profileIconId\": 663,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 8,\n" +
> > > " \"infPoints\": 534204,\n" +
> > > " \"revisionDate\": 1480719827000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 1281203,\n" +
> > > " \"accountId\": 1259342,\n" +
> > > " \"name\": \"AC Klondike\",\n" +
> > > " \"profileIconId\": 898,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 27,\n" +
> > > " \"infPoints\": 191429,\n" +
> > > " \"revisionDate\": 1480294973000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 13161471,\n" +
> > > " \"accountId\": 207847181,\n" +
> > > " \"name\": \"adar21\",\n" +
> > > " \"profileIconId\": 26,\n" +
> > > " \"level\": 10,\n" +
> > > " \"expPoints\": 143,\n" +
> > > " \"infPoints\": 3558,\n" +
> > > " \"revisionDate\": 1476529855000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 5841915,\n" +
> > > " \"accountId\": 202998794,\n" +
> > > " \"name\": \"Achilles29\",\n" +
> > > " \"profileIconId\": 666,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 41,\n" +
> > > " \"infPoints\": 219714,\n" +
> > > " \"revisionDate\": 1480777744000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 2853062,\n" +
> > > " \"accountId\": 200726707,\n" +
> > > " \"name\": \"AbIanStarBebegim\",\n" +
> > > " \"profileIconId\": 909,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 64,\n" +
> > > " \"infPoints\": 297580,\n" +
> > > " \"revisionDate\": 1480556859000\n" +
> > > " },\n" +
> > > " {\n" +
> > > " \"id\": 8323114,\n" +
> > > " \"accountId\": 205093515,\n" +
> > > " \"name\": \"Absuruk\",\n" +
> > > " \"profileIconId\": 6,\n" +
> > > " \"level\": 30,\n" +
> > > " \"expPoints\": 21,\n" +
> > > " \"infPoints\": 121086,\n" +
> > > " \"revisionDate\": 1480820801000\n" +
> > > " }\n" +
> > > "]"
> > >
> > > On Wed, Dec 7, 2016 at 7:01 AM, Hannes Wallnöfer
> > > <hannes.wallnoe...@oracle.com> wrote:
> > > Hi Jesus,
> > >
> > > I’m trying to reproduce the problem, and just want to make sure I get the
> > > missing pieces right.
> > >
> > > You already showed us how you’re setting up the engine and the JS code
> > > you’re running. I assume the JSON code you’re parsing is a simple array
> > > of objects? And you’re just calling Invocable.invokeFunction on the
> > > ScriptEngine from multiple threads in parallel, right?
> > >
> > > Thanks,
> > > Hannes
> > >
> > >
> > > > Am 07.12.2016 um 00:03 schrieb Jesus Luzon <jlu...@riotgames.com>:
> > > >
> > > > When we share one invocable across many threads and run invokeFunction
> > > > it
> > > > happens, such as this:
> > > >
> > > > ExecutorService executor = Executors.newFixedThreadPool(50);
> > > >>
> > > >> Invocable invocable = generateInvocable(script);
> > > >>
> > > >> AtomicLong count = new AtomicLong();
> > > >>
> > > >> for (int i = 0; i < 50; i++) {
> > > >>
> > > >> executor.submit(new Runnable() {
> > > >>
> > > >> @Override
> > > >>
> > > >> public void run() {
> > > >>
> > > >> try {
> > > >>
> > > >> while(true) {
> > > >>
> > > >> invocable.invokeFunction("transform",
> > > >>> something);
> > > >>
> > > >> count.incrementAndGet();
> > > >>
> > > >> }
> > > >>
> > > >> } catch (NoSuchMethodException | ScriptException
> > > >>> e) {
> > > >>
> > > >> e.printStackTrace();
> > > >>
> > > >> }
> > > >>
> > > >> }
> > > >>
> > > >> });
> > > >>
> > > >> }
> > > >>
> > > >>
> > > >
> > > >
> > > > On Tue, Dec 6, 2016 at 2:59 PM, Jim Laskey (Oracle)
> > > > <james.las...@oracle.com
> > > >> wrote:
> > > >
> > > >> Intersting. The example you posted demonstrates this behaviour? If so
> > > >> I’ll file a bug and dig in. It sounds like an object is being reused
> > > >> across invocations and accumulating changes to the property map.
> > > >>
> > > >> — Jim
> > > >>
> > > >>
> > > >> On Dec 6, 2016, at 5:12 PM, Jesus Luzon <jlu...@riotgames.com> wrote:
> > > >>
> > > >> With more threads you are impacting the same 8 cores, so it will taper
> > > >> off
> > > >>> after 8 threads. If it’s a 2x4 core machine then I can see 4 being a
> > > >>> threshold depending on System performance. Transport: I meant if you
> > > >>> were
> > > >>> using sockets to provide the script.
> > > >>
> > > >> This makes sense. This one's on me then.
> > > >>
> > > >>
> > > >>> So you are using the same invocable instance for all threads? If so,
> > > >>> then you are probably good to go. As far as leaks are concerned, not
> > > >>> sure
> > > >>> how you would get leaks from Nashorn. The JSON object is written in
> > > >>> Java,
> > > >>> and little JavaScript involved.
> > > >>
> > > >>
> > > >>
> > > >>> In your example, pull up Invocable invocable =
> > > >>> generateInvocable(script);
> > > >>> out of the loop and use the same invocable for all threads.
> > > >>
> > > >>
> > > >> We were using one invocable across all threads and we were getting
> > > >> slowdowns on execution, high CPU Usage and memory leaks that led to
> > > >> OutOfMemory errors. I could trace the leak to
> > > >>
> > > >> jdk.nashorn.internal.objects.Global -> *objectSpill* Object[8] ->
> > > >> jdk.nashorn.internal.scripts.JO4 -> *arrayData*
> > > >> jdk.nashorn.internal.runtime.arrays.SparseArraysData -> *underlying*
> > > >> jdk.nashorn.internal.runtime.arrays.DeletedArrayFilter
> > > >>
> > > >> which just keeps growing forever.
> > > >>
> > > >> On Tue, Dec 6, 2016 at 6:30 AM, Jim Laskey (Oracle) <
> > > >> james.las...@oracle.com> wrote:
> > > >>
> > > >>>
> > > >>> On Dec 6, 2016, at 9:56 AM, Jesus Luzon <jlu...@riotgames.com> wrote:
> > > >>>
> > > >>> The cost of creating a new engine is significant. So share an engine
> > > >>>> across threads but use *eval
> > > >>>> <https://docs.oracle.com/javase/7/docs/api/javax/script/ScriptEngine.html#eval(java.lang.String,%20javax.script.ScriptContext)>*
> > > >>>> (String
> > > >>>> <https://docs.oracle.com/javase/7/docs/api/java/lang/String.html>
> > > >>>> script, ScriptContext
> > > >>>> <https://docs.oracle.com/javase/7/docs/api/javax/script/ScriptContext.html>
> > > >>>> context) instead, separate context per execution. If your JavaScript
> > > >>>> code does not modify globals you can get away with using the same
> > > >>>> engine,
> > > >>>> same compiled script on each thread.
> > > >>>
> > > >>>
> > > >>> I guess there's a few things here I don't understand. One thing I'm
> > > >>> trying to do is sharing a CompiledScript (which is why I'm using
> > > >>> invocable). Also, what exactly does modify globals mean? All our
> > > >>> filters do
> > > >>> the same thing, make a function that takes a JSON String, turns it
> > > >>> into a
> > > >>> JSON, modifies it and then stringifies it back. No state is changed of
> > > >>> anything else but there are temporary vars created inside the scope
> > > >>> of the
> > > >>> function. When we run this multithreaded, running invokeFunction
> > > >>> slows down
> > > >>> significantly and we get crazy memory leaks.
> > > >>>
> > > >>>
> > > >>> So you are using the same invocable instance for all threads? If so,
> > > >>> then you are probably good to go. As far as leaks are concerned, not
> > > >>> sure
> > > >>> how you would get leaks from Nashorn. The JSON object is written in
> > > >>> Java,
> > > >>> and little JavaScript involved.
> > > >>>
> > > >>>
> > > >>> Of course there are many factors involved n performance. How many
> > > >>> cores
> > > >>>> do you have on the test machine? How much memory in the process?
> > > >>>> What
> > > >>>> transport are you using between threads? That sort of thing. Other
> > > >>>> than
> > > >>>> constructing then engine and context Nashorn performance should
> > > >>>> scale.
> > > >>>
> > > >>> I'm using an 8 core machine to test with 2.5Gs of RAM allocated to the
> > > >>> process. Not sure what transports between threads means, but this is
> > > >>> the
> > > >>> code I'm benchmarking with. Increasing the number of threads actually
> > > >>> makes
> > > >>> it go faster until about 4 threads, then adding more threads takes
> > > >>> the same
> > > >>> amount to get to 1000 and and after a certain point it is just slower
> > > >>> to
> > > >>> get to 1000 counts. Some of our filters need to be able to run over
> > > >>> 1000
> > > >>> times a second (across all threads) and the fastest time I could
> > > >>> actually
> > > >>> get with this was about 2.4 seconds for a 1000 counts.
> > > >>>
> > > >>>> ExecutorService executor = Executors.newFixedThreadPool(50);
> > > >>>>
> > > >>>> AtomicLong count = new AtomicLong();
> > > >>>>
> > > >>>> for (int i = 0; i < 50; i++) {
> > > >>>>
> > > >>>> executor.submit(new Runnable() {
> > > >>>>
> > > >>>> @Override
> > > >>>>
> > > >>>> public void run() {
> > > >>>>
> > > >>>>
> > > >>>>> try {
> > > >>>>
> > > >>>> Invocable invocable =
> > > >>>>> generateInvocable(script);
> > > >>>>
> > > >>>> while(true) {
> > > >>>>
> > > >>>> invocable.invokeFunction("transform",
> > > >>>>> something);
> > > >>>>
> > > >>>> count.incrementAndGet();
> > > >>>>
> > > >>>> }
> > > >>>>
> > > >>>> } catch (NoSuchMethodException |
> > > >>>> ScriptException
> > > >>>>> e) {
> > > >>>>
> > > >>>> e.printStackTrace();
> > > >>>>
> > > >>>> }
> > > >>>>
> > > >>>> }
> > > >>>>
> > > >>>> });
> > > >>>>
> > > >>>> }
> > > >>>>
> > > >>>> long lastTimestamp = System.currentTimeMillis();
> > > >>>>
> > > >>>> while(true) {
> > > >>>>
> > > >>>>
> > > >>>>> if (count.get() > 1000) {
> > > >>>>
> > > >>>> count.getAndAdd(-1000);
> > > >>>>
> > > >>>> System.out.println((System.currentTimeMillis() -
> > > >>>>> lastTimestamp)/1000.0);
> > > >>>>
> > > >>>> lastTimestamp = System.currentTimeMillis();
> > > >>>>
> > > >>>> }
> > > >>>>
> > > >>>> }
> > > >>>>
> > > >>>>
> > > >>> With more threads you are impacting the same 8 cores, so it will taper
> > > >>> off after 8 threads. If it’s a 2x4 core machine then I can see 4
> > > >>> being a
> > > >>> threshold depending on System performance. Transport: I meant if you
> > > >>> were
> > > >>> using sockets to provide the script.
> > > >>>
> > > >>> In your example, pull up Invocable invocable =
> > > >>> generateInvocable(script);
> > > >>> out of the loop and use the same invocable for all threads.
> > > >>>
> > > >>> - Jim
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>> On Tue, Dec 6, 2016 at 5:31 AM, Jim Laskey (Oracle) <
> > > >>> james.las...@oracle.com> wrote:
> > > >>>
> > > >>>>
> > > >>>> On Dec 6, 2016, at 9:19 AM, Jesus Luzon <jlu...@riotgames.com> wrote:
> > > >>>>
> > > >>>> Hey Jim,
> > > >>>>
> > > >>>> I looked at it and I will look into loadWithNewGlobal to see what
> > > >>>> exactly it does since it could be relevant. As for the rest, for my
> > > >>>> use
> > > >>>> case having threads in the JS would not help. We're using Nashorn to
> > > >>>> build
> > > >>>> JSON filters in a Dynamic Proxy Service and need any of the threads
> > > >>>> processing a request to be able to execute the script to filter.
> > > >>>>
> > > >>>>
> > > >>>> The cost of creating a new engine is significant. So share an engine
> > > >>>> across threads but use *eval
> > > >>>> <https://docs.oracle.com/javase/7/docs/api/javax/script/ScriptEngine.html#eval(java.lang.String,%20javax.script.ScriptContext)>*
> > > >>>> (String
> > > >>>> <https://docs.oracle.com/javase/7/docs/api/java/lang/String.html>
> > > >>>> script, ScriptContext
> > > >>>> <https://docs.oracle.com/javase/7/docs/api/javax/script/ScriptContext.html>
> > > >>>> context) instead, separate context per execution. If your JavaScript
> > > >>>> code does not modify globals you can get away with using the same
> > > >>>> engine,
> > > >>>> same compiled script on each thread.
> > > >>>>
> > > >>>>
> > > >>>> Also, when you say a new engine per threads is the worst case what
> > > >>>> exactly do you mean? I would expect an initial cost of compiling the
> > > >>>> script
> > > >>>> on each thread and then each engine should be able to do its own
> > > >>>> thing, but
> > > >>>> what I'm seeing is that when running with more than 10 threads all my
> > > >>>> engines get slow at executing code, even though they are all
> > > >>>> completely
> > > >>>> separate from each other.
> > > >>>>
> > > >>>>
> > > >>>> Of course there are many factors involved n performance. How many
> > > >>>> cores
> > > >>>> do you have on the test machine? How much memory in the process?
> > > >>>> What
> > > >>>> transport are you using between threads? That sort of thing. Other
> > > >>>> than
> > > >>>> constructing then engine and context Nashorn performance should
> > > >>>> scale.
> > > >>>>
> > > >>>>
> > > >>>> On Tue, Dec 6, 2016 at 5:07 AM, Jim Laskey (Oracle) <
> > > >>>> james.las...@oracle.com> wrote:
> > > >>>>
> > > >>>>> Jesus,
> > > >>>>>
> > > >>>>> Probably the most informative information is in this blog.
> > > >>>>>
> > > >>>>> https://blogs.oracle.com/nashorn/entry/nashorn_multi_threading_and_mt
> > > >>>>>
> > > >>>>> This example uses Executors but threads would work as well.
> > > >>>>>
> > > >>>>> I did a talk that looked at different methods to max out
> > > >>>>> multithreading
> > > >>>>> performance. A new engine per thread is the worst case. A new
> > > >>>>> context per
> > > >>>>> thread does much better. A new global per thread is the best while
> > > >>>>> remaining thread safe.
> > > >>>>>
> > > >>>>> Cheers,
> > > >>>>>
> > > >>>>> — Jim
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> On Dec 6, 2016, at 8:45 AM, Jesus Luzon <jlu...@riotgames.com>
> > > >>>>> wrote:
> > > >>>>>
> > > >>>>> Hey folks,
> > > >>>>>
> > > >>>>> I've tried many different ways of using Nashorn multithreaded based
> > > >>>>> on
> > > >>>>> what
> > > >>>>> I've found on the internet and I still can't get a single one to
> > > >>>>> scale.
> > > >>>>> Even the most naive method of making many script engines with my
> > > >>>>> script
> > > >>>>> tends to bottleneck itself when I have more than 10 threads invoking
> > > >>>>> functions.
> > > >>>>>
> > > >>>>> I'm using the following code to compile my script and
> > > >>>>> invocable.invokeFunction("transform", input) to execute:
> > > >>>>>
> > > >>>>> static Invocable generateInvocable(String script) throws
> > > >>>>> ScriptException {
> > > >>>>> ScriptEngineManager manager = new ScriptEngineManager();
> > > >>>>> ScriptEngine engine =
> > > >>>>> manager.getEngineByName(JAVASCRIPT_ENGINE_NAME);
> > > >>>>> Compilable compilable = (Compilable) engine;
> > > >>>>> final CompiledScript compiled = compilable.compile(script);
> > > >>>>> compiled.eval();
> > > >>>>> return (Invocable) engine;
> > > >>>>> }
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> The script I'm compiling is:
> > > >>>>>
> > > >>>>> String script = "function transform(input) {" +
> > > >>>>> "var result = JSON.parse(input);" +
> > > >>>>> "response = {};\n" +
> > > >>>>> "for (var i = 0; i < result.length; i++) {\n" +
> > > >>>>> " var summoner = {};\n" +
> > > >>>>> " summoner.id = result[i].id;\n" +
> > > >>>>> " summoner.name = result[i].name;\n" +
> > > >>>>> " summoner.profileIconId =
> > > >>>>> result[i].profileIconId;\n" +
> > > >>>>> " summoner.revisionDate =
> > > >>>>> result[i].revisionDate;\n" +
> > > >>>>> " summoner.summonerLevel = result[i].level;\n" +
> > > >>>>> " response[summoner.id] = summoner;\n" +
> > > >>>>> "}\n" +
> > > >>>>> "result = response;" +
> > > >>>>> "return JSON.stringify(result);" +
> > > >>>>> "};";
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> I've also tried other more scaleable ways to work with scripts
> > > >>>>> concurrently, but given that this is the most naive method where
> > > >>>>> everything
> > > >>>>> is brand new and I still get slowness calling them concurrently I
> > > >>>>> fear
> > > >>>>> that
> > > >>>>> maybe I'm overlooking something extremely basic on my code.
> > > >>>>>
> > > >>>>> Thanks.
> > > >>>>> -Jesus Luzon
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>
> > > >>>
> > > >>
> > > >>
> > >
> > >
> > >
> >
> >
> >
>
>
>
