GH-2864 SHACL benchmarks use NativeStore to simulate more realistic usage #2882
Conversation
…istic usage, also some code cleanup while we were at it Signed-off-by: Håvard Ottestad <[email protected]>
|
|
||
| @Override | ||
| public synchronized Throwable fillInStackTrace() { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Not sure if this is a generated file. This is essentially a small performance improvement for code that generates a lot of SPARQL queries.
There was a problem hiding this comment.
It is a generated file, as indicated by the comment at the top, so this will be overwritten whenever we next generate the parser from the grammar.
I'm also not sure what the consequences of this change are. What happens on errors in sparql parsing? Are we suddenly leaving out neccessary information from some SPARQL parser errors? Why is this even a performance consideration (I am assuming that this only happens when there is a parse error?)?
There was a problem hiding this comment.
It's the parser abusing exceptions for control flow. Not a parser error.
| baseSail.setValueCacheSize(100); | ||
| baseSail.setNamespaceCacheSize(100); | ||
| baseSail.setValueIDCacheSize(100); | ||
| baseSail.setNamespaceIDCacheSize(100); |
There was a problem hiding this comment.
If you want realistic usage benmarking, should you be tweaking these settings? I doubt many of our users do.
| + "> ?age. FILTER(datatype(?age) != <http://www.w3.org/2001/XMLSchema#int>)}") | ||
| .evaluate() | ||
| .stream()) { | ||
| stream.forEach(System.out::println); |
There was a problem hiding this comment.
Won't this generate rather a lot of noise on the console?
There was a problem hiding this comment.
Didn't catch that one.
|
@hmottestad this has a few conflicts, probably needs a rebase - other than that I guess this is good to be merged? |
|
I've actually got cold feet about this one. I think I'll try to give in a look over the weekend and see if there were some fixes I wanted to keep and then revert the rest. |
GitHub issue resolved: #2864
Briefly describe the changes proposed in this PR:
PR Author Checklist (see the contributor guidelines for more details):
mvn process-resourcesto format from the command line)