export errors and reduce box usage

[xMAC94x]

Hey :)

I did another PR focusing on execution speed and Errors.

• extended the JsonPathParserError error crate with more custom things to get rid of the plain-String errors. This way its easier for libraries to handle errors and catch them.
• switch the boolean parsing to a own function rather than invoking another json parse.
• put the pest error in a box (because its 270 bytes long, which means that the Result is very large, even in the good case, because we always need to reserve those bytes to be prepared for the bad case)

• The function json_path_instance returned a Box<dyn> which means that the type is determined on runtime, and this is compiled via a vtable. As all posibilities are known i just created a simple enum containing those and implementing the Path trait. This allows the compiler to further optimize the crate.

  There are a few drawbacks of this change we should consider:

  • I hope i got the Error messages correct, I dont know the parser too well, but i hope they make sense
  • We no longer export the Pairs in Error directly, because the lifetimes made problems with TryFrom trait, so for now its just the String representation of the Pairs
  • I hope the custom str_to_bool function can never fail, IMO the path is only Path::boolean if the str is either true or false as described in the .pest file, but I have never worked with that parse, is this assumption true?
  • to avoid String generation in parse_logic_or and parse_logic_and functions, i did a check on the len of that iterator. IMO as soon as there is 1 entry, the result cannot be None is this correct ? Otherwise I would have introduced a logical bug, but this way we only prepare the Error message on demand`.

So this change needs some good code-review before we just merge it, i don't want to introduce some logic bugs.

On the other hand, the compilation time for a JsonPathInst is quite a bit faster now, from over 70 us to under 15us.

I was also thinking about JsonPath and JsonPathInst, maybe its possible to just use the JsonPath in the future, and also move the 3 find methods into this struct. This would definitly be something for another PR, but tell me what you think about it ? Because this PR contains some breaking changes, so we should prob wait with a new release for a bit after merging this.

[besok]

Hi! Thank you for the PR. The improvements are excellent.

I will take a look at it in the evening

[besok]

I hope i got the Error messages correct, I dont know the parser too well, but i hope they make sense

Yeah, everything seems to be correct.

We no longer export the Pairs in Error directly, because the lifetimes made problems with TryFrom trait, so for now its just the String representation of the Pairs

That is fine

I hope the custom str_to_bool function can never fail, IMO the path is only Path::boolean if the str is either true or false as described in the .pest file, but I have never worked with that parse, is this assumption true?

yeah, that is correct, we can so to say guarantee that it will work.

to avoid String generation in parse_logic_or and parse_logic_and functions, i did a check on the len of that iterator. IMO as soon as there is 1 entry, the result cannot be None is this correct ? Otherwise I would have introduced a logical bug, but this way we only prepare the Error message on demand`.

yeah, that is correct. At least one atom should be presented.

On the other hand, the compilation time for a JsonPathInst is quite a bit faster now, from over 70 us to under 15us.

That is very impressive

I was also thinking about JsonPath and JsonPathInst, maybe its possible to just use the JsonPath in the future, and also move the 3 find methods into this struct. This would definitly be something for another PR, but tell me what you think about it ? Because this PR contains some breaking changes, so we should prob wait with a new release for a bit after merging this.

Fair enough.

Great job!