I should have said I was thinking of the reimplementation as a temporary measure with the assumption that the VM make_hash2 would be exported at some point and we could throw away our version. But the phash2 option sounds much better.
Nick On Mon, 13 Jul 2015 at 18:41, Paul Davis <[email protected]> wrote: > It might be possible to reimplement make_hash2 but it feels a bit > dirty, not to mention we'd have to assign someone to watching Erlang > commits in case they ever change the implementation for any reason. > > As for calling phash2 overhead, I don't think it'll be significant. We > have to do the hash either way, so its basically just the overhead > from calling a BIF which doesn't even register on the radar for things > that we think about for performance. > > On Mon, Jul 13, 2015 at 12:11 PM, Nick North <[email protected]> wrote: > > This feels conceptually nicer than shipping a modified VM. Any idea what > > effect it would have on performance? > > > > Alternatively, putting my deep ignorance of the VM to use to make silly > > suggestions: would it be possible to replicate (or even just copy the > code > > for) make_hash2? That would save the term_to_binary requirement, but > maybe > > make_hash2 needs knowledge from the inside of the VM, which might make > the > > idea impossible. > > > > Nick > > > > On Sun, 12 Jul 2015 at 23:32 Robert Samuel Newson <[email protected]> > > wrote: > > > >> How about this; > >> > >> 1) When built on Windows, the khash_hash_fun insists that the key term > is > >> an erlang binary and we hash that. > >> > >> 2) When built on Windows, the functions in khash.erl call term_to_binary > >> before calling down to the NIF functions. > >> > >> B. > >> > >> sketch; > >> > >> #ifndef _WIN32 > >> hash_val_t > >> khash_hash_fun(const void* obj) > >> { > >> khnode_t* node = (khnode_t*) obj; > >> return (hash_val_t) make_hash2(node->key); > >> } > >> #else > >> hash_val_t > >> khash_hash_fun(const void* obj) > >> { > >> khnode_t* node = (khnode_t*) obj; > >> Eterm term = node->key; > >> if (ERL_IS_BINARY(term)) { > >> void* data = ERL_BIN_PTR(term); > >> int len = ERL_BIN_SIZE(term); > >> return hash(data, len); // TODO, write hash function > >> } > >> else { > >> return (hash_val_t) make_hash2(node->key); > >> } > >> } > >> #endif > >> > >> > >> > On 12 Jul 2015, at 09:14, Nick North <[email protected]> wrote: > >> > > >> > I'm with Dave - it would be acceptable to ship a modified BEAM if > it's a > >> > short-term measure. It's not great, but better than not having Windows > >> > support. > >> > > >> > Nick > >> > > >> > On Sun, 12 Jul 2015 at 08:35 Dave Cottlehuber <[email protected]> wrote: > >> > > >> >> > >> >> On Sun, 12 Jul 2015, at 09:21 AM, Dave Cottlehuber wrote: > >> >>> On Sun, 12 Jul 2015, at 08:35 AM, Joan Touzet wrote: > >> >>>> I scoured the mailing lists but was unable to find any sort of > patch > >> >>>> for this, submitted or not. It's certainly not landed in v18.0.x. > >> >>>> > >> >>>> Perhaps Paul is aware of the patch as a starting place at least? > >> >> > >> >> Um I should also say that in the distant past, I built & shipped > >> >> modified BEAMs, although that was just to fix up patches that had > been > >> >> accepted but not released. e.g. > >> >> http://people.apache.org/~dch/snapshots/1.1.1/ > >> >> > >> >> So I'm not averse to that if we have a patch that's been submitted > >> >> upstream and a reasonable expectation of success. In my experience > there > >> >> are only a handful of people who have built from source on Windows... > >> >> > >> >> — > >> >> Dave Cottlehuber > >> >> [email protected] > >> >> Sent from my Couch > >> >> > >> > >> >
