Skip to content

LibZstd Clang.jl generated bindings #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 5, 2021
Merged

LibZstd Clang.jl generated bindings #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d076afa
Initial attempt at Clang generated API calls
mkitti Sep 16, 2021
47753a9
Generate zstd_api with ZSTD_STATIC_LINKING_ONLY
mkitti Sep 16, 2021
9416204
Add patch versus generated output
mkitti Sep 16, 2021
702a164
Amend gitignore
mkitti Sep 16, 2021
397c6fc
Change module name from Lib to LibZstd
mkitti Sep 16, 2021
bd3469a
Use Clang 0.14 generation
mkitti Sep 16, 2021
6976631
Remove empty lines
mkitti Sep 16, 2021
a81136e
Add patch for mutable buffer structs
mkitti Sep 16, 2021
c084472
Tidy up commented out code
mkitti Sep 16, 2021
e52b2c2
Use rewrite! mechanism rather than patch
mkitti Sep 17, 2021
ca42fe4
Add Project.toml, Manifest.toml, directions
mkitti Sep 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
/deps/build.log
/deps/deps.jl
/deps/usr/
src/libzstd/LibTemplate.jl
src/libzstd/ctypes.jl
1 change: 1 addition & 0 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ authors = ["Kenta Sato <bicycle1885@gmail.com>"]
version = "0.7.0"

[deps]
CEnum = "fa961155-64e5-5f13-b03f-caf6b980ea82"
TranscodingStreams = "3bb67fe8-82b1-5028-8e26-92a6c54297fa"
Zstd_jll = "3161d3a3-bdf6-5164-811a-617609db77b4"

Expand Down
1 change: 1 addition & 0 deletions src/CodecZstd.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import TranscodingStreams:

using Zstd_jll

include(joinpath("libzstd","LibZstd.jl"))
include("libzstd.jl")
include("compression.jl")
include("decompression.jl")
Expand Down
72 changes: 36 additions & 36 deletions src/libzstd.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,24 @@
# ====================

function iserror(code::Csize_t)
return ccall((:ZSTD_isError, libzstd), Cuint, (Csize_t,), code) != 0
return LibZstd.ZSTD_isError(code) != 0
end

function zstderror(stream, code::Csize_t)
ptr = ccall((:ZSTD_getErrorName, libzstd), Cstring, (Csize_t,), code)
ptr = LibZstd.ZSTD_getErrorName(code)
error("zstd error: ", unsafe_string(ptr))
end

function max_clevel()
return ccall((:ZSTD_maxCLevel, libzstd), Cint, ())
return LibZstd.ZSTD_maxCLevel()
end

const MAX_CLEVEL = max_clevel()

# ZSTD_outBuffer
mutable struct InBuffer
src::Ptr{Cvoid}
size::Csize_t
pos::Csize_t

function InBuffer()
return new(C_NULL, 0, 0)
end
end

# ZSTD_inBuffer
mutable struct OutBuffer
dst::Ptr{Cvoid}
size::Csize_t
pos::Csize_t

function OutBuffer()
return new(C_NULL, 0, 0)
end
end
const InBuffer = LibZstd.ZSTD_inBuffer
InBuffer() = InBuffer(C_NULL, 0, 0)
const OutBuffer = LibZstd.ZSTD_outBuffer
OutBuffer() = OutBuffer(C_NULL, 0, 0)

# ZSTD_CStream
mutable struct CStream
Expand All @@ -45,7 +28,7 @@ mutable struct CStream
obuffer::OutBuffer

function CStream()
ptr = ccall((:ZSTD_createCStream, libzstd), Ptr{Cvoid}, ())
ptr = LibZstd.ZSTD_createCStream()
if ptr == C_NULL
throw(OutOfMemoryError())
end
Expand All @@ -54,23 +37,38 @@ mutable struct CStream
end

function initialize!(cstream::CStream, level::Integer)
return ccall((:ZSTD_initCStream, libzstd), Csize_t, (Ptr{Cvoid}, Cint), cstream.ptr, level)
return LibZstd.ZSTD_initCStream(cstream.ptr, level)
end

function reset!(cstream::CStream, srcsize::Integer)
return ccall((:ZSTD_resetCStream, libzstd), Csize_t, (Ptr{Cvoid}, Culonglong), cstream.ptr, srcsize)
# ZSTD_resetCStream is deprecated
# https://github.com/facebook/zstd/blob/9d2a45a705e22ad4817b41442949cd0f78597154/lib/zstd.h#L2253-L2272
res = LibZstd.ZSTD_CCtx_reset(cstream.ptr, LibZstd.ZSTD_reset_session_only)
if iserror(res)
return res
end
if srcsize == 0
# From zstd.h:
# Note: ZSTD_resetCStream() interprets pledgedSrcSize == 0 as ZSTD_CONTENTSIZE_UNKNOWN, but
# ZSTD_CCtx_setPledgedSrcSize() does not do the same, so ZSTD_CONTENTSIZE_UNKNOWN must be
# explicitly specified.
srcsize = ZSTD_CONTENTSIZE_UNKNOWN
end
return LibZstd.ZSTD_CCtx_setPledgedSrcSize(cstream.ptr, srcsize)
#return ccall((:ZSTD_resetCStream, libzstd), Csize_t, (Ptr{Cvoid}, Culonglong), cstream.ptr, srcsize)

end

function compress!(cstream::CStream)
return ccall((:ZSTD_compressStream, libzstd), Csize_t, (Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cvoid}), cstream.ptr, pointer_from_objref(cstream.obuffer), pointer_from_objref(cstream.ibuffer))
return LibZstd.ZSTD_compressStream(cstream.ptr, pointer_from_objref(cstream.obuffer), pointer_from_objref(cstream.ibuffer))
end

function finish!(cstream::CStream)
return ccall((:ZSTD_endStream, libzstd), Csize_t, (Ptr{Cvoid}, Ptr{Cvoid}), cstream.ptr, pointer_from_objref(cstream.obuffer))
return LibZstd.ZSTD_endStream(cstream.ptr, pointer_from_objref(cstream.obuffer))
end

function free!(cstream::CStream)
return ccall((:ZSTD_freeCStream, libzstd), Csize_t, (Ptr{Cvoid},), cstream.ptr)
return LibZstd.ZSTD_freeCStream(cstream.ptr)
end

# ZSTD_DStream
Expand All @@ -80,7 +78,7 @@ mutable struct DStream
obuffer::OutBuffer

function DStream()
ptr = ccall((:ZSTD_createDStream, libzstd), Ptr{Cvoid}, ())
ptr = LibZstd.ZSTD_createDStream()
if ptr == C_NULL
throw(OutOfMemoryError())
end
Expand All @@ -89,19 +87,21 @@ mutable struct DStream
end

function initialize!(dstream::DStream)
return ccall((:ZSTD_initDStream, libzstd), Csize_t, (Ptr{Cvoid},), dstream.ptr)
return LibZstd.ZSTD_initDStream(dstream.ptr)
Copy link

@yuyichao yuyichao Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do NOT do these object to pointer conversions manually. Leave it to ccall to do it. I’m assuming clang isn’t generating a signature that requires you to do such conversion manually. If that’s not the case then the code it generate is broken and must be fixed. ( a brief look suggests that the generated wrappers are correct)

same with all uses of pointer_from_objref here.

Copy link

@yuyichao yuyichao Sep 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I just realized that the alignment of the diff made me think that the change is new to this pr but it’s actually not….

I that case it should still be fixed but doesn’t have to be done here.

Copy link
Member Author

@mkitti mkitti Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear what you are suggesting here.

LibZstd.ZSTD_initDStream now calls ccall. All I'm doing with the change above is consolidating all the ccall references into the LibZstd module.

CodecZstd.jl/src/libzstd/LibZstd.jl

Lines 307 to 309 in c084472

function ZSTD_initDStream(zds)
ccall((:ZSTD_initDStream, libzstd), Csize_t, (Ptr{ZSTD_DStream},), zds)
end

Perhaps you mean that I should just pass dstream directly rather than dstream.ptr or something like that?

Copy link

@yuyichao yuyichao Sep 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the change you made is fine. But both the new and the old version should have the Julia object (dsteam in this case) directly passed to ccall. You’ll need to define the correct unsafe convert to make it work.

Copy link
Member Author

@mkitti mkitti Sep 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean. This probably has some garbage collection implications as well now that I think about it.

I think I might want to save those changes for another PR and perhaps another release.

Copy link

@yuyichao yuyichao Sep 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason it’s broken is indeed because of object lifetime.

end

function reset!(dstream::DStream)
return ccall((:ZSTD_resetDStream, libzstd), Csize_t, (Ptr{Cvoid},), dstream.ptr)
# LibZstd.ZSTD_resetDStream is deprecated
# https://github.com/facebook/zstd/blob/9d2a45a705e22ad4817b41442949cd0f78597154/lib/zstd.h#L2332-L2339
return LibZstd.ZSTD_DCtx_reset(dstream.ptr, LibZstd.ZSTD_reset_session_only)
end

function decompress!(dstream::DStream)
return ccall((:ZSTD_decompressStream, libzstd), Csize_t, (Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cvoid}), dstream.ptr, pointer_from_objref(dstream.obuffer), pointer_from_objref(dstream.ibuffer))
return LibZstd.ZSTD_decompressStream(dstream.ptr, pointer_from_objref(dstream.obuffer), pointer_from_objref(dstream.ibuffer))
end

function free!(dstream::DStream)
return ccall((:ZSTD_freeDStream, libzstd), Csize_t, (Ptr{Cvoid},), dstream.ptr)
return LibZstd.ZSTD_freeDStream(dstream.ptr)
end


Expand All @@ -112,5 +112,5 @@ const ZSTD_CONTENTSIZE_UNKNOWN = Culonglong(0) - 1
const ZSTD_CONTENTSIZE_ERROR = Culonglong(0) - 2

function find_decompressed_size(src::Ptr, size::Integer)
return ccall((:ZSTD_findDecompressedSize, libzstd), Culonglong, (Ptr{Cvoid}, Csize_t), src, size)
return LibZstd.ZSTD_findDecompressedSize(src, size)
end
Loading