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
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>
> > > >>>
> > > >>
> > > >>
> > >
> > >
> > >
> >
> >
> >
>
>
