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 >> >> >> >>
