Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix off by one in out of memory test #96

Merged
merged 1 commit into from Mar 6, 2017
Merged

fix off by one in out of memory test #96

merged 1 commit into from Mar 6, 2017

Conversation

rahulpalamuttam
Copy link
Collaborator

The outofmemory test was failing for me.
Not sure if this is the intended fix.

Otherwise, we might want to change
this line in lib.rs
to be >= instead of >.

@jjthomas jjthomas merged commit e38b2dd into weld-project:master Mar 6, 2017
@jjthomas
Copy link
Collaborator

jjthomas commented Mar 6, 2017

actually wait @sppalkia this suggests there might be a bigger problem on Linux right ... this should exceed the memory limit even with 50000/4 due to allocations for the output structures?
my guess is that maybe the thread isn't aborting when weld_rt_malloc exceeds the limit?

@rahulpalamuttam what is the err_value from the program when the size is 50000/4?

@rahulpalamuttam
Copy link
Collaborator Author

@jjthomas
Hmm okay looks it might not be so simple.

With 50000/4, the program does enter the body of the if statement in weld_rt_malloc.

I'm not able to find out what the err_value is because compile_and_run_error fails with
FATAL: exception not rethrown.

@jjthomas
Copy link
Collaborator

jjthomas commented Mar 6, 2017

yeah seems like calling pthread_cancel is unreliable: http://stackoverflow.com/questions/34972909/fatal-exception-not-rethrown-in-c

the strange thing is that we call pthread_exit and not pthread_cancel: http://man7.org/linux/man-pages/man3/pthread_exit.3.html

which is supposed to exit the thread right away. Anyway @rahulpalamuttam can you try adding a sleep() call (or whatever the rust equivalent is) after the weld_abort_thread() call in weld_rt_malloc?

@rahulpalamuttam
Copy link
Collaborator Author

rahulpalamuttam commented Mar 6, 2017

It doesn't get to the sleep call - looks like the thread terminates at pthread-exit.

Also I'm noticing weld_rt_malloc being called twice - is the second time because it allocates the result?
Edit:
Actually, it's being called 3 times.
The first time it successfully allocates 50k.
mem_info.mem_allocated + (size as i64) = 50,000
The second time it tries another 50k but fails.
mem_info.mem_allocated + (size as i64) = 100,000
^^ This is the strange part - why is it trying to allocate another 50k?
The third time it tries 24 and fails.
mem_info.mem_allocated + (size as i64) = 50,024

@jjthomas
Copy link
Collaborator

jjthomas commented Mar 6, 2017

Yeah so the reason it's allocating the second 50k is that the multicore-aware vecmerger allocates a new output buffer when result is called and copies all of the per-thread buffers into it (in this case the copy is wasteful since we only have on thread).

So I guess the weird thing is that the thread keeps running even after the second 50k allocation, right? So it must reach the sleep() call that occurs after the weld_abort_thread call from the second allocation?

jjthomas added a commit that referenced this pull request Mar 7, 2017
jjthomas added a commit that referenced this pull request Mar 7, 2017
* Revert "Update Docs (#79)"

This reverts commit 2ea8fd7.

* Revert "fix off by one in out of memory test (#96)"

This reverts commit e38b2dd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants