JuliaCrypto / Nettle.jl

Julia wrapper around nettle cryptographic hashing/encryption library providing MD5, SHA1, SHA2 hashing and HMAC functionality, as well as AES encryption/decryption
Other
51 stars 34 forks source link

Rewrite to be more precompilable. API BREAKAGE! #52

Closed staticfloat closed 9 years ago

staticfloat commented 9 years ago

@stevengj I took the opportunity to rewrite basically the whole thing in a much more precompile-friendly manner. On this branch, the time of julia -e 'using Nettle' looks like:

master sf/breakageahoy
Julia 0.3 1.721s 1.456s
Julia 0.4 1.436s 0.554s

If I actually hash something (julia -e 'using Nettle; md5_hash("a")' on master, julia -e 'using Nettle; hash!("md5", "a")' on this branch), there is a difference:

master sf/breakageahoy
Julia 0.3 1.726s 1.560s
Julia 0.4 1.457s 0.642s

I am a little disappointed that hash!() doesn't seem to be getting precompiled; I tried using SnoopCompile.jl, and have included the results of my efforts here, but I don't seem to be getting what I want, @time output still looks an awful lot like what I would expect from slamming into codegen on first run:

$ time ./julia -e 'using Nettle; @time hash!("md5", "a"); @time hash!("md5", "a")'
  0.087612 seconds (70.92 k allocations: 3.103 MB)
  0.000007 seconds (9 allocations: 544 bytes)

real    0m1.101s
user    0m1.094s
sys     0m0.111s

@timholy do you have any tips for how to better get this initial runtime down? For the record, I have done the following:

using SnoopCompile
using Nettle
@snoop1 "test.csv" hash!("md5", "a")
data = SnoopCompile.read("test.csv")
pc, discards = SnoopCompile.parcel(data[end:-1:1,2])
SnoopCompile.write("/tmp/precompile", pc)

Then when I cat /tmp/precompile/precompile_Nettle.jl, I get:

function _precompile_()
    ccall(:jl_generating_output, Cint, ()) == 1 || return nothing
    precompile(Nettle.call, (Type{Nettle.NettleHashType}, ASCIIString, UInt32, UInt32, UIn
t32, Ptr{Void}, Ptr{Void}, Ptr{Void}, Ptr{Void},))
end

This was after running SnoopCompile on the entire include(Pkg.dir("Nettle", "test", "runtests.jl")) thing, since this time around I wanted to see if anything had been missed. Since it only came up with a single file, I figured that must be the only line I needed to add to my precompile.jl, but things just don't seem to be working out like I expected them to.

staticfloat commented 9 years ago

On a completely unrelated note; this PR completely breaks the API, since we don't want to be autogenerating function names. I'm pretty happy with what I have here, with the exception of hash!(), which is probably going to be the most-called function!

The rest of the similarly-named functions are called things like hmac() or encrypt(), but because hash is both a noun and a verb, I find it very likely that users will want to use a variable called hash, and won't appreciate the fact that Nettle's hash conflicts with their usage, hence, hash!(). Thoughts? Should I:

staticfloat commented 9 years ago

@stevengj I have addressed all your comments and decided to go the calc_hash() and calc_hmac() routes, especially since otherwise Base.hash() conflicts with Nettle.hash().

stevengj commented 9 years ago

Is there any reason why most of the operations are limited to ASCIIStrings? I would think you would want to support ByteString and Vector{UInt8} at least, and maybe Vector{T} for any bitstype T.

Oh nevermind, I guess I was just looking at the precompile calls?

stevengj commented 9 years ago

Instead of calc_hash, why not just define:

digest!(state::Hasher, data) = digest!(update!(Hasher, data))
digest(hashname::AbstractString, data) = digest!(Hasher(hashname), data)
staticfloat commented 9 years ago

That is such a straightforward idea, I'm flabbergasted I didn't think of it before.

staticfloat commented 9 years ago

I think this is ready now. @stevengj, would you like me to hold off on merging this into master until you've had a chance to get things ready on the IJulia side of things, or should I fire at will?

stevengj commented 9 years ago

Fire at will.

As long as you hold off on tagging until IJulia is ready, I'm not concerned about a short time of incompatibility on master.

stevengj commented 9 years ago

Maybe file issues with Faker.jl, GnuTLS.jl, OAuth.jl, WebSockets.jl, and Yelp.jl to let them know of the API changes?

staticfloat commented 9 years ago

Thanks, that list was helpful. Where did you get it? Can pkg.julialang.org do reverse-depends or something? -E

On Mon, Sep 21, 2015 at 1:38 PM, Steven G. Johnson <notifications@github.com

wrote:

Maybe file issues with Faker.jl, GnuTLS.jl, OAuth.jl, WebSockets.jl, and Yelp.jl to let them know of the API changes?

— Reply to this email directly or view it on GitHub https://github.com/staticfloat/Nettle.jl/pull/52#issuecomment-142101894.

stevengj commented 9 years ago

No, I run the following shell script to fetch all registered Julia packages, then I just grep Nettle */REQUIRE:

#!/bin/sh

for f in ~/.julia/v0.5/METADATA/*/url; do
    p=$(basename $(dirname $f))
    url=`cat $f`
    echo "updating $p from $url..."
    if test ! -d $p; then
        git clone $url $p
    else
        (cd $p; git pull origin master)
    fi
done
stevengj commented 9 years ago

I've tagged a new version of IJulia that is compatible with both the current Nettle version and the previous version, so it will be fine whenever you tag a new Nettle.