Re: Bug? R: Org babel block execution *drastically* slower than in ESS session directly

[Eric Schulte <schulte.eric@gmail.com>]

Eric Schulte <schulte.eric@gmail.com> writes:

> Aaron Ecay <aaronecay@gmail.com> writes:
>
>> 2012ko azaroak 17an, Eric Schulte-ek idatzi zuen:
>>
>>> Oh!, thanks for catching this, I just pushed up a fix.
>>
>> This is no longer a (complete) fix for the original problem, though.  (A
>> large part of) the slowdown comes from reading the results from a temp
>> file and transforming them into an elispy format, which is handled by
>> the backends.  Your code only prevents them from being echoed to the
>> minibuffer.
>>
>> You can see this by trying your original “seq” tests.  You should see
>> them taking the same (very long) amount of time.
>>
>> If you set debug-on-quit to t (before evaluating the block) and
>> interrupt emacs with C-g when it hangs, you’ll see a backtrace that goes
>> through ‘org-babel-execute:sh’ and then
>> ‘org-babel-import-elisp-from-file’.  The presence of the former in the
>> call stack is why I claim that per-backend fixes are ultimately needed.
>> The latter is where my patch addresses the problem in a temporary way.
>>
>
> I apologize for rushing through your previous email and missing
> important portions of the content.
>
>>
>>> 
>>> I may be outvoted, but I find this approach too be overly complicated.
>>> Also, size may frequently not be the best proxy for the time which
>>> Emacs will take to ingest the results.
>>
>> I agree that my patch is a stopgap.  But until per-backend fixes are
>> available, this allows certain code to run that otherwise wouldn’t (at
>> least not without hacks, such as putting NULL at the end of an R source
>> block so that the “.Last.value” in the block is trivial).
>
> I'm attaching a new patch which should serve as a more permanent
> solution, however, given that it touches almost every backend I want to
> give the list a chance to try it out before committing it.  This patch
> introduces a new macro (`org-babel-result-cond') which attempts to unify
> the results processing.  Not only does this check for ":results none"
> and ignore all results in that case, but it also checks whether the
> results should be parsed as a scalar or a vector.  Hopefully this will
> serve as useful refactoring and reduce the total amount of code as well
> as provide for more uniform results processing across languages.
>
> Please let me know if this looks like a good permanent solution.
>
> I see that I'm now getting 5 test failures with this patch applied
> locally, but I won't have time to look at this further until early next
> week at the earliest, so I'm sharing the existing patch now.
>
> Thanks,

Alright, I've now changed the above patch so that as many tests are
passing locally as were passing without the patch.  Given that, I've
applied this patch to the main git repository.  Everything which worked
previously should still be working, only now the ":results none" options
should have the desired effect.

Cheers,

-- 
Eric Schulte
http://cs.unm.edu/~eschulte