KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
697 stars 230 forks source link

Delete leaves file in memory. #409

Closed TDW89 closed 9 years ago

TDW89 commented 9 years ago

Not sure if this counts as a bug or enhancement.

LOG "PRINT 1." TO temp.
RUN temp.
DELETE temp.
LOG "PRINT 2." TO temp.
RUN temp.

Should print a 1 then a 2 but becouse temp has already been run and is loaded into memory (I think this is the reason) then the next time it goes to run temp it runs the same as it did the first time. Is it possible to have the Delete comand force kOS to always reload the Deleted script next time it is called?

Edit:spelling

erendrake commented 9 years ago

@TDW89 I didnt reproduce the error here but i found a suspicious bit of code that was incorrect and fixed it for 0.16.0. Would you mind trying again with the new build and letting us know if that resolves it?

TDW89 commented 9 years ago

@erendrake will have a look this evening but may not be able to give feedback right away as i will be out of the country at that point and unsure of WiFi. Edit: I will check back for build v0.16.0 in a bit. Not sure how to build it myself (especially on a mobile).

erendrake commented 9 years ago

@TDW89 thats ok, we havent released the 'fix' yet. check back in a few days. I just did want to forget to ping you about it :)

TDW89 commented 9 years ago

@erendrake The above script still prints;

1
1

in kOS 0.16.2.0 instead of;

1
2

Also quick thought on the solution method. It may be better to have the log command remove the script from memory as opposed to the delete command (if that is what is causing the issue) as the log command is the one doing the modification. This way the script

LOG "PRINT 1." TO temp.
RUN temp.
\\ DELETE temp.
LOG "PRINT 2." TO temp.
RUN temp.

would print

1
1
2
Dunbaratu commented 9 years ago

@erendrake, I suspect the nature of the problem was slightly misnamed. I think the FILE, and its in-memory representation if it's on a local volume, were being changed just fine. That wasn't the problem, it sounds like.

The problem seems to be very specific to the case where LOG is being used to perform the self-modifying code trick rather than being used to make a passive bunch of text in a log file.

Consider the following small example:

// this is the script called "outer.ks"
// -------------------------------------
set x to 0.
until x >= 100 {
  run inner(x).
  set x to x + 1.
}.
// this is the script called "inner.ks"
declare parameter val
print "inner script was passed the value " + val.

In that example, outer.ks runs inner.ks 100 times, and you get a printout like so:

inner script was passed the value 0
inner script was passed the value 1
inner script was passed the value 2
inner script was passed the value 3
inner script was passed the value 4
... etc....

On the surface of it, it looks like it would inefficiently re-run the very expensive process of parsing, 100 times - each time outer.ks issues the command RUN INNER(X). But kOS isn't THAT wasteful. It's smart enough to know that the second time within a program run that it's being asked to JIT-compile and run the same script it already compiled, it can skip the compiling step and just run the same opcodes that were already built the first time, that it hasn't purged from the program context yet.

The result of the previous compile of the script isn't purged until the outermost program finishes and you're dumped back to the interactive prompt. Only then does it re-compile a script if it is asked to run it again.

@TDW89, when you run this:

LOG "PRINT 1." TO temp.
RUN temp.
\\ DELETE temp.
LOG "PRINT 2." TO temp.
RUN temp.

Are you running those commands from a script, or interactively by typing them one at a time into the terminal window?

My suspicion, if my guess as to the cause of this is correct, is that you'll get a different result when you run them interactively one at a time from what you get when they're run from a script.

When run interactively, the program "temp.ks" is the outermost program, so when it finishes it's previous compile result is purged and the next time it's recompiled from scratch. When run from a script, the program "temp.ks" is a script nested inside another script, which means it's previous compile's result is NOT purged when it finishes, and instead gets re-used (for the reason stated above - recompiling a RUN command in a loop over and over would be wasteful in scenarios where you'd expect the program to be the same as it was the last time because a user hasn't had a chance to edit the script between iterations of the loop. Parsing is an expensive activity.)

I'm not sure what the solution should be. I definitely don't want to take on the expense of recompiling every time. I want you to be able to RUN from inside a loop and have it be fast. Perhaps we could have RUN take a look at the file timestamp and force a recompile if the timestamp is newer than the previous compile, although that would still incur a slight expense when the file is on the archive, because getting that timestamp means actually physically querying the filesystem for it.

TDW89 commented 9 years ago

@Dunbaratu Yes your analysis of the issue sounds spot on. Sorry I should have made that clearer, I have been running that as a script not as individual commands. When typed in separately you get the result you would expect. The issue only occurs when running the same script multiple times from another script. The reasoning behind why this happens make sense. If the only way round it causes kOS significant expence its probably not worth it for a marginal case.

TDW89 commented 9 years ago

@Dunbaratu Had a thought on this at work, don’t know how hard it would be to implement as I don’t understand the underlying code of kOS. (I actually wondered if this had been solved as a side effect of file extensions / .ksm and unspecified files and I just hadn't noticed.) When is the check for "have I previously compiled this script"? does it happen around the same time that kOS checks (when there is no specified file type) for a .ksm file? If so would amalgamating the two checks work. So that;

RUN temp.

would run in order of preference [ previously compiled > .ksm > .ks ]. Where as;

RUN temp.ks.

would say no I want you to recompile from the .ks file.

erendrake commented 9 years ago

@Dunbaratu I suspect you are correct :) i assumed that it was a file management issue.

As far as a solution, Could we have the DELETE command clear out the compiled information. effectively forcing a recompile the next time you run it?

Dunbaratu commented 9 years ago

I like the Delete solution I will consider that

TDW89 commented 9 years ago

@erendrake is there a reason for using the delete command to do this over the log command (other than the issue title)? Case for using log to clear the compiled info being that log is making a change to the file where as delete is getting rid of it.

Dunbaratu commented 9 years ago

@TDW89 you have a point - not just with the LOG command but just with kOS file I/O in general: We have control over when and how a kerboscript can alter a file's contents on the filesystem. We can probably find one or two "chokepoints" in the code where all the file alterations are funneled through, and just make it mark the file as "dirty" in a way the RUN command can read and know the file needs an unconditional recompile and it should bypass its "did I already compile this" check. That way we're not checking timestamps out on the filesystem itself. We're just checking a boolean flag on our wrapper class that abstracts the on-disk file.

This won't catch a case where an external program altered the file while the script was running, but you could argue that that's a case where you wouldn't want it to, and you'd rather have it keep running on the old compile until re-run.

erendrake commented 9 years ago

after finishing a very similar post, @Dunbaratu beat me to it :) I obviously agree

abenkovskii commented 9 years ago

It looks like you are already storing FileInfo:MODIFIED so you can just add timestamp to the cached version of program like real compilers do.