Conversation
|
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 |
| ["data"] = null, | ||
| }; | ||
|
|
||
| [TestMethod] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
My bad, the second test shouldn't pass because of the depth limit.
| results.AddRange(objects.Where(p => p is not null).SelectMany(p => p.Properties).Where(p => p.Key == token.Content).Select(p => p.Value)); | ||
| Descent(ref objects, ref maxDepth); | ||
| Descent(ref objects, ref maxDepth, maxObjects); | ||
| if (results.Count > maxObjects) throw new InvalidOperationException(nameof(maxObjects)); |
There was a problem hiding this comment.
I would suggest to make the exception more precise, use ArgumentOutOfRangeException or OutOfMemoryException instead.
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.
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. |
|
@fyrchik love your solution, may you please share with us the time it costs to run this |
|
@shargon hey shargon, do you have any plan to further optimise the jsonpath filter? |
|
@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 So, 1 minute execution time is certainly not what we want. |
We can reduce the max filter length, and the max depth to 4. |
|
@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, |
|
@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. |
|
@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. |
* Add ToJson overload (#2671) * Add ToJson overload * change * Update src/neo/VM/Helper.cs * Update src/neo/VM/Helper.cs * Update src/neo/VM/Helper.cs * Update src/neo/VM/Helper.cs Co-authored-by: Jinghui Liao <jinghui@wayne.edu> * Update src/neo/VM/Helper.cs Co-authored-by: Jinghui Liao <jinghui@wayne.edu> Co-authored-by: Shargon <shargon@gmail.com> Co-authored-by: Jinghui Liao <jinghui@wayne.edu> * Fix oom (#2665) * Fix oom * Revert reorder * parameters order Co-authored-by: Erik Zhang <erik@neo.org> * Optimize inventory (#2659) * add `murmur32` to crypto lib (#2604) * 3.2.0 * fix Co-authored-by: Shargon <shargon@gmail.com> Co-authored-by: Jinghui Liao <jinghui@wayne.edu>
Fix #2663