Fix oom

[shargon]

Fix #2663

[fyrchik]

This solution restricts intermediate array size, even though the final output can be empty (e.g. the last level index can be out of range). I have implemented another solution (process value in a DFS fashion and restrict the size the real result to MaxOracleResponseSize), but this one is simpler to implement and covers all reasonable cases. Good job!

[fyrchik]

Could you also add a boundary test where the size of an intermediate array is approximately equal to the maximum allowed limit?

The size in case is width^depth (64^6 in this case). So tests for 2^10 (width 10, depth 2) and 32^2 (width 2, depth 32) should pass and result in an array of 1024 empty objects. Note, that the value itself should also be changed in this case (amount of [ should be equal to depth).

This will help to ensure neo-go stays compatible.

[fyrchik]

My bad, the second test shouldn't pass because of the depth limit.

[Jim8y]

I would suggest to make the exception more precise, use ArgumentOutOfRangeException or OutOfMemoryException instead.

[shargon]

I have implemented another solution (process value in a DFS fashion and restrict the size the real result to MaxOracleResponseSize)

Could you share your solution?

[fyrchik]

Could you share your solution?

Here it is nspcc-dev/neo-go@e4ec405 .

The idea is that we first parse the filter itself into some struct, which allows us to perform DFS on the value we apply filter on.
Good things about it:

1. JSON can be formed simultaneously with traversal, avoiding need to construct objects in memory (haven't finished this yet, because of purely technical difficulties).
2. It can return the correct answer in more cases.

However, there is a single bad thing which makes me like your solution more: my implementation leaves room for filters which take long time to process. And this can't be reflected in GAS fee. I can't come up with an example immediately, but it could exist in theory.

[Jim8y]

@fyrchik love your solution, may you please share with us the time it costs to run this oom case when you fully implement it in dfs?

[Jim8y]

@shargon hey shargon, do you have any plan to further optimise the jsonpath filter? JsonPath should not process such an empty or simple json at all in this oom case, maybe we can have a preprocess or something to verify both the json and filter before executing it.

[fyrchik]

@Liaojinghui the remaining part won't change anything significantly, sadly I also don't have time to work on finishing this. However, some results can already be seen. I took the example from this issue, replaced the last [0,0... with [1,1... and benchmarked againt different depth parameters.

depth  time
4      22.766441ms
5      1.142487091s
6      1m18.027576585s

So, 1 minute execution time is certainly not what we want.

[shargon]

So, 1 minute execution time is certainly not what we want.

We can reduce the max filter length, and the max depth to 4.

[fyrchik]

@shargon but should we? In your solution it is easy to see why the time is bounded: we move through the filter in one direction,
maintain the list of current objects and don't allow this list to grow large. IMO limiting max depth can prevent some reasonable applications from working, while limiting intermediate objects amount achieve exactly what we want: do not process large or time-consuming inputs.

[Jim8y]

@fyrchik how do we deal with a reasonable application but will cause oom? No matter how reasonable it is, the core problem here is we do not have the ability or enough memory to process it.

And size limit can be seen everywhere in the virtual machine.

[fyrchik]

@Liaojinghui I think we agree on the issue. The choice here is between restricting the amount of intermediate objects and further restricting maximum allowed depth. They serve the same purpose, so I see less value in restricting both of these parameters.
But with the former (this PR) we will be able to support a wider class of reasonable applications.