Re: Constructive criticism

new topic     » goto parent     » topic index » view thread      » older message » newer message
Critic said...

Your example is quite typical EU code:

Thank you.

Critic said...
  • Relies on a global variable (I refer to lifetime here, not visibility!) "stacks", because of lack of PBR

So what? It is owned by the module its in and is private. What exactly is it a problem if this sequence exists for the entire life of the run-time?

Critic said...
  • Because of this a "delete" operation is needed that the user code needs to call when done with a stack; otherwise the result is a memory leak! This should be totally unnecessary in a language with GC because the stack is cleary not an external resource.

Yes. This is an issue. We are working on a generic release management solution that will cover that aspect.

On the other hand, using the 'delete' is no more a problem than closing files. We do that without bitching about it. A deleted stack takes up 4-bytes. I can live with that.

Critic said...
  • Common subexpression elimination is impossible to do efficiently in EU. Lets try with this code:
public procedure rot(integer stack)  
    object temp  
  
    validate(stack)  
    if length(stacks[stack]) > 2 then  
        temp = stacks[stack][$]  
        stacks[stack][$] = stacks[stack][$-1]  
        stacks[stack][$-1] = stacks[stack][$-2]  
        stacks[stack][$-2] = temp  
    end if  
end procedure  

Hm, it sucks to refer to "stacks[stack]" all the time, but avoiding this introduces unnecessary sequence copies:

public procedure rot(integer stack)  
    object temp 
 
    validate(stack) 
    object s = stacks[stack] -- we do not copy here: COW 
    if length(s) > 2 then  
        temp = s[$] 
        s[$] = s[$-1]        -- Ooops: here s is copied completely! 
        s[$-1] = s[$-2]  
        s[$-2] = temp 
         
        stacks[stack] = s    -- COW 
    end if 
end procedure  

The first thing to note is that you have reminded me of that optimisation technique. Your version of the routine takes half the time than my original one. Even though you seem to be saying that it should be slower, it is fact much faster.

Secondly, the "common subexpression elimination" problem is of two types. The first is on a purely textual basis. I will be advocating a method of allowing some controlled textual substistutions in future versions of Euphoria. Something along the lines of ...

public procedure rot(integer stack)  
    object temp  
  
    validate(stack)  
    alias stacks[stack] as s 
    if length(s) > 2 then  
        temp = s[$]  
        s[$] = s[$-1]  
        s[$-1] = s[$-2]  
        s[$-2] = temp  
    end if  
end procedure  

This would "generate" the same code as my original version.

Further more, there is still many more optimisations that we can put into both the interpreter and translator that should allow caching of subexpressions. This would cause such code as above to automatically run as if it was coded more like your version.

And finally, it is anticipated that a 'swap' operator will be available in 4.1 that will further optimise this type of routine.

public procedure rot(integer stack)  
    validate(stack)  
    alias stacks[stack] as s 
    if length(s) > 2 then  
        s[$]  <=> s[$-1]  
        s[$-1] <=> s[$-2]  
    end if  
end procedure  
Critic said...

So an optimization that appears in the manual (!) makes the algorithm O(n) instead of O(1). Ah, the "efficiency" and "elegance" of Euphoria. smile

Try benchmarking it yourself and you will see that this is not so. The suggested optimisation works well.

new topic     » goto parent     » topic index » view thread      » older message » newer message

Search



Quick Links

User menu

Not signed in.

Misc Menu