search

alavrik / piqi

Piqi – universal schema language: JSON, XML, Protocol Buffers data validation and conversion
http://piqi.org
Apache License 2.0
246 stars 36 forks source link

Eliminate non-tail recursive calls as source of stack exhaustion errors #51

Closed KennethAdamMiller closed 9 years ago

KennethAdamMiller commented 9 years ago

So, I didn't have time to make sure and remove Core in favor of Core_kernel-that's the only drawback. But I was relentless in chasing down this bug. Holy crap did this take a long time. I'm pretty sure that there is literally no instance of pervasives' List.map. I think I'll have to make a later pass for memory leaking, but for now, it's not blowing up due to stack exhaustion.

alavrik commented 9 years ago

It looks like we needed to replace just a couple of functions to their tail-recursive versions. So I just did that. Please see my latest change. I like it better because it doesn't require adding another dependency and changing the whole codebase to use its conventions. Let me know if this works for you.

alavrik commented 9 years ago

Made another change replacing remaining and potentially useful List functions with tail-recursive implementations: https://github.com/alavrik/piqi/commit/98cd2b59c3b52b7e3e351c7bbe7d406fbf5cae2c

KennethAdamMiller commented 9 years ago

Yeah, it's not building with just strictly the latest commits from your head. I have to revert shortly to my own to continue testing on my ends. I see in Travis CI that your builds are working, that's great, but here's my error:

File "piqi_common.ml", line 327, characters 2-6: Parse error: [semi] expected after [str_item](in [implem])

(* surrounding code of line 327 *) let trace_indent = ref 0

let print_trace_indent () = for i = 1 to !trace_indent do prerr_string " " done ...

alavrik commented 9 years ago

try running make distclean && ./configure && make deps && make && make ocaml. it looks like the optcomp dependency ise missing or didn't get rebuilt after upgrading the compiler/camlp4.

KennethAdamMiller commented 9 years ago

I think something like that happened with camlp4 because opam flipped the hell out unreasonably. I think yesterday someone pushed an incompatible version requirement for LWT and it caused all kinds of issues when pinning and unpinning CMU's BAP. In any case, I was extra perplexed when I saw the Travis CI build working fine for that code. It works now, so whatever.

On Tue, Jun 30, 2015 at 3:33 AM, Anton Lavrik notifications@github.com wrote:

try running make distclean && ./configure && make deps && make && make ocaml. it looks like the optcomp dependency ise missing or didn't get rebuilt after upgrading the compiler/camlp4.

— Reply to this email directly or view it on GitHub https://github.com/alavrik/piqi/pull/51#issuecomment-117037700.