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

Heap memory limit #2296

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Heap memory limit #2296

wants to merge 4 commits into from

Conversation

bakaq
Copy link
Contributor

@bakaq bakaq commented Jan 14, 2024

Adds a simple check for the size of the heap that is done every once in a while, as described in #2294. I haven't implemented the cli argument yet, but it should be possible with this. Example usage:

?- [user].
get_heap_limit(Lim) :- '$get_heap_limit'(Lim).
set_heap_limit(Lim) :- '$set_heap_limit'(Lim).

?- get_heap_limit(Lim).
   % No limit by default.
   Lim = no_limit.
?- get_heap_limit(no_limit).
   true.
?- set_heap_limit(100000).
   true.
?- get_heap_limit(Lim).
   Lim = 100000.
?- get_heap_limit(no_limit).
   false.
?- set_heap_limit(not_integer).
   error(type_error(integer,'$set_heap_limit'/1),'$set_heap_limit'/1).
?- set_heap_limit(no_limit).
   % I have not implemented this, so there is no way to "unset" the limit.
   % Should this be implemented?
   error(type_error(integer,'$set_heap_limit'/1),'$set_heap_limit'/1).
?- M is 10^6, length(L, M).
   % Clean resource error. I'm not sure what to put in the second argument,
   % so I made it a variable. I would appreciate suggestions.
   error(resource_error(heap_limit,100000),_42).
?- 

@UWN
Copy link

UWN commented Jan 14, 2024

ulrich@gupu:/opt/gupu/bakaq-scryer-prolog$ target/release/scryer-prolog
?- [user].
get_heap_limit(Lim) :- '$get_heap_limit'(Lim).
set_heap_limit(Lim) :- '$set_heap_limit'(Lim).


?- set_heap_limit(100000).
   thread 'main' panicked at 'index out of bounds: the len is 87987 but the index is 8935141660703064304', src/machine/dispatch.rs:545:24
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
ulrich@gupu:/opt/gupu/bakaq-scryer-prolog$ ulimit -v
unlimited
ulrich@gupu:/opt/gupu/bakaq-scryer-prolog$ target/release/scryer-prolog-v
bash: target/release/scryer-prolog-v: No such file or directory
ulrich@gupu:/opt/gupu/bakaq-scryer-prolog$ target/release/scryer-prolog -v
v0.9.3-175-g003b95df

@bakaq
Copy link
Contributor Author

bakaq commented Jan 14, 2024

I can't reproduce. At least not with such a high value.

.../repos/dev/scryer-prolog> ./target/release/scryer-prolog -f                                                                                                                    heap_memory_limit
?- [user].
get_heap_limit(Lim) :- '$get_heap_limit'(Lim).
set_heap_limit(Lim) :- '$set_heap_limit'(Lim).

?- set_heap_limit(100000).
   true.
?- set_heap_limit(10000).
   true.
?- set_heap_limit(1000).
   thread 'main' panicked at src/machine/dispatch.rs:545:33:
index out of bounds: the len is 24061 but the index is 8935141660703064368
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
.../repos/dev/scryer-prolog> ./target/release/scryer-prolog -v                                                                                                                heap_memory_limit 101
v0.9.3-175-g003b95df

@bakaq
Copy link
Contributor Author

bakaq commented Jan 14, 2024

Ok, I can reproduce without the -f option. It seems loading modules is causing some weird things here.

@bakaq
Copy link
Contributor Author

bakaq commented Jan 15, 2024

It seems to be a problem with backtracking. I think it's doing the backtracking even though there is no frame from where it could load, so I think it hits Undefined Behavior inside RawBlock and loads some instruction out of some place it shouldn't. This seems really hard to fix, and I think I didn't do anything wrong with the backtracking interface of Machine.

@UWN
Copy link

UWN commented Jan 15, 2024

(One reason why you were not able to reproduce it with exactly the same values is that you invoked Scryer with a minimally different path: target/release/scryer-prolog vs. ./target/...)

@mthom
Copy link
Owner

mthom commented Jan 19, 2024

The crash happens because check_heap_limit is called a second time, during the propagation of the first check_heap_limit exception, and self.machine_st.block = 0 the second time, having been reset by the first. If a hard heap limit is being set, shouldn't check_heap_limit reset the heap length to the limited amount prior to writing the exception to the heap? But even then, depending on the severity of the limit, the same problem may appear again as the heap fills during the exception propagation.

Throwing resource errors is one of the problems I'm now working on by adding a proper GC, replacing the current heap data type of Vec<HeapCellValue> with a custom block with low-level allocations. What this PR is trying to do is extremely tricky and hazardous. It may be best to wait for the GC to be completed. If @bakaq can make it work in the meantime, by all means, but as I said, it's very easy to run into such obstacles.

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.

3 participants