FourierTransformer / lua-simdjson

simdjson bindings for lua
Apache License 2.0
14 stars 8 forks source link

bump simdjson to version 3.2.3 #59

Closed striezel closed 1 month ago

striezel commented 1 year ago

This pull request updates simdjson to 3.2.3, the most recent version.

Closes #61. Closes #60. Closes #57. Closes #56.

@FourierTransformer: You possibly want to merge #58 first to make sure that lua-simdjson still builds properly on CI.

striezel commented 11 months ago

@FourierTransformer: I've just rebased this pull request on top of the current master branch and updated simdjson from version 3.2.1 to version 3.2.3.

FourierTransformer commented 11 months ago

I've actually been playing around with this lately, and it seems like the json pointer code can cause segfaults. I haven't figured out if it's related to the new version yet, but overall it doesn't seem great.

striezel commented 11 months ago

I've actually been playing around with this lately, and it seems like the json pointer code can cause segfaults.

Can you provide more details on that? A small, reproducible example would also be helpful.

FourierTransformer commented 11 months ago

I noticed it when I first started updating some of the unit tests to 3.3.0. I have a draft pr/branch out here: https://github.com/FourierTransformer/lua-simdjson/pull/63 Looks like it's failing in GHA as well with the new tests. The optimization level doesn't seem to affect it. I ran the unit tests and it seemed to work on the old version of the code just fine. I'm going to assume some of the internals changed in the past 3 major versions... heh...

rough copy from the interpreter:

simdjson = require("simdjson")
function loadFile(textFile)
    local file = io.open(textFile, "r")
    if not file then error("File not found at " .. textFile) end
    local allLines = file:read("*all")
    file:close()
    return allLines
end
demo = loadFile("jsonexamples/small/demo.json")
djson = simdjson.open(demo)
djson:atPointer("/Image/Width")
simdjson.openFile("jsonexamples/small/demo.json")
Segmentation fault (core dumped)
FourierTransformer commented 1 month ago

closing in favor of #83

striezel commented 1 month ago

So the segfaults you mentioned are fixed in #83, I assume? That's good news. :)

FourierTransformer commented 1 month ago

Yup! The new[er] simdjson ondemand parser, you need to keep the parser, document, and string in scope and it wasn't quite doing that >.<