No alloc#132
Conversation
|
this is now lying around since two weeks, anyone had a chance to look at it, and would like to share their opinion? this is a blocker (/precondition) for other refactorings I've in preparation, such as not throwing exceptions. |
|
LGTM |
|
|
||
| let marshal txbuf dns = | ||
| let marshal dns = | ||
| let txbuf = Cstruct.create 4096 in |
There was a problem hiding this comment.
It's this Cstruct.create that could potentially cripple performance for a caller, since there is now choice but to allocate a page for every packet. Previously, the caller of the interface would control memory allocation and could reuse pages from a free pool if it was safe.
Why not just make txbuf an optional function that can be passed in, that does the allocation (defaulting to Cstruct.create)?
OCaml doesn't deal particularly well with rapid page allocation via Bigarrays, since there is also a C-level proxy object that (unnecessarily in our use case) reference counts the memory, so I'm reluctant to make ocaml-dns force memory allocation on the caller unless there's a really good reason.
|
@avsm I re-added a EDIT: adjusted the lwt and async layer, made tests build again, canceled 5 travis jobs. ready to merge imho |
|
lgtm; @djs55 is having a review also and then will merge/release. |
|
LGTM also. |
async: add backlog argument to serve
Include packages that needed CLOCK API updates.
[new release] uutf (1.0.3+dune)
4 participants
rebased #128 on master, and removed the allocation interface. see mirage/mirage-net-xen#17 for reasoning why page-aligned allocation is not needed.
the allocation strategy is modified (API is changed), this may also lead to different memory behaviour.