search

privacylab / talek

a Private Publish Subscribe System
BSD 2-Clause "Simplified" License
47 stars 5 forks source link

Use new PIR implementations in current code base #60

Closed willscott closed 7 years ago

willscott commented 7 years ago

These changes replace the previous pird implementation with the choice of the three pir implementations introduced in ryscheng-pir. It retains the previous libpir interface for the moment to keep the extent of the changes localized.

ryscheng commented 7 years ago

Looks like you didn't properly vendor github.com/prometheus/common/log Check out the README.md to see how to use govendor.

We already have a similar logger in common.Logger. Is it necessary to pull in another dependency to do this? I think we should stick to 1 logging convention

willscott commented 7 years ago

whoops. unintentional. removed that dep

willscott commented 7 years ago

we also each did our own

//+build tag

rules. do we want opencl and cuda to be opt-in or opt-out? you did opt-out, i did opt-in. i think we need the build rule in both places, because full tests will look into the sub directory, but the files in the /pir directory will be used to create the conditional dependency on the module.

willscott commented 7 years ago

should the default kernel be "cpu.0" vs 1 or 2?

ryscheng commented 7 years ago

Re: build tags I'd personally prefer opt-out. I'd like to build everything when we can and have to explicitly remove code. As opposed to opt-in where we can easily forget to test before committing/pushing.

Re: CPU version. I kept 3 versions for our benchmarks. If you run go test -bench=. in the pircpu/ directory you'll see which is fastest (cpu.0) BenchmarkShardCPUReadv0-8 1 11015218932 ns/op BenchmarkShardCPUReadv1-8 1 11297790850 ns/op BenchmarkShardCPUReadv2-8 1 64850486195 ns/op

ryscheng commented 7 years ago

👍