Optimize performance of TkText#value= when prefixing/suffixing

[AndyObtiva]

The TkText#value= method is very convenient for simpler quick usage than tinkering with insert/delete manually.

I just optimized it to avoid doing a pre-delete when suffixing or prefixing the previous value (only adding a suffix or prefix to existing value)

[jeremyevans]

Thanks for working on this. Have you benchmarked the difference in this case? I'm not sure how expensive the delete and insert are compared to the additional Ruby code. I guess if they are expensive, this could double the speed for large TkText values?

In terms of further optimization:

• value can be saved to a local variable, instead of calling the method multiple times.
• You can use the non-range form of String#[] (the range form is slower as it allocates a range)

[AndyObtiva]

OK, I ran a manual test comparing performance of original code to pull request's code performing prefixing/suffixing/bulk-prefixing/bulk-suffixing/doubling/reset of a very large text value in Tk::Text using the following Glimmer DSL for Tk code:

require 'glimmer-dsl-tk'

class TestText
  include Glimmer
  
  DOCUMENT = <<~MULTI_LINE_STRING
    According to the National Post, a heavy metal-loving high school principal in Canada will be allowed to keep her job, days after a public campaign to oust her made headlines around the globe.
    
    Parents at Eden High School in St. Catharines, Ontario launched a petition to remove principal Sharon Burns after discovering she's an IRON MAIDEN fan.
    
    The petition, titled "Eden High School Principal, Sharon Burns, Needs to Be Transferred Immediately!" read, "We are deeply disturbed that the principal assigned to the school blatantly showed Satanic symbols and her allegiance to Satanic practices on her public social media platforms where all the students can see them under @edenprincipal (not her personal account)."
    
    More than 500 people signed the petition to transfer Burns after she posted a picture of herself flashing the "devil horns" hand sign with a doll of the MAIDEN zombie mascot Eddie behind her as well as a personalized license plate reading "IRNMADEN" and a handwritten note that reads "Eddie 666" on a car's dashboard.
    
    
    The number 666 is used to represent the devil, and is featured prominently in MAIDEN's artwork by the band, whose classic third album is titled "The Number Of The Beast".
    
    The petition was later updated to state that the demand for her transfer is not because of her love for metal, but for "openly displaying her own handmade sign with the 666 clearly displayed on it".
    
    The creator of the original petition wrote: "Sharon knows full well what she did was simply inappropriate, unnecessary and not professional but has yet to publicly admit so and is willing to allow people to believe a completely different story, making very real concerns seem petty."
    
    Meanwhile, a counter-petition supporting Burns garnered over 20,000 signatures.
    
    "It is ridiculous that a couple of parents judge her role as a principal only based on an Instagram post. (About liking the band IRON MAIDEN. That's it.) Eden High School is a public school. Not a Christian school," the petition titled "We need Mrs Burns" stated. "She has made Eden a safe space for so many people. She spreads nothing but love and kindness."
    
    After the complaints were aired, the District School Board of Niagara spoke with Burns and the parents who launched the petition, and the issue is over as far as the board is concerned, Kim Sweeney, the board's chief communications officer, told the National Post. No disciplinary action or policy changes were needed.
    
    "Our belief is that taste in music is subjective and we support that both students and staff enjoy a wide variety of genres," Sweeney said.
    
    The original petition has since been removed.
  MULTI_LINE_STRING
  VALUE = DOCUMENT*10000
  
  def launch
    root {
      title 'Hello, Text!'
      width 1280
      height 800
      
      frame {
        grid row: 0, column: 0
        
        column_index = -1
        
        button {
          grid row: 1, column: column_index += 1, column_weight: 0
          text 'Prefix'
          
          on('command') do
            @text.value = "Prefix#{Time.now.to_f}#{@text.value}"
          end
        }
        
        button {
          grid row: 1, column: column_index += 1, column_weight: 0
          text 'Bulk Prefix'
          
          on('command') do
            @text.value = "#{1000.times.map {"Prefix#{Time.now.to_f}"}.join}#{@text.value}"
          end
        }
        
        button {
          grid row: 1, column: column_index += 1, column_weight: 0
          text 'Suffix'
          
          on('command') do
            @text.value = "#{@text.value}Suffix#{Time.now.to_f}"
          end
        }
        
        button {
          grid row: 1, column: column_index += 1, column_weight: 0
          text 'Bulk Suffix'
          
          on('command') do
            @text.value = "#{@text.value}#{1000.times.map {"Suffix#{Time.now.to_f}"}.join}"
          end
        }
        
        button {
          grid row: 1, column: column_index += 1, column_weight: 0
          text 'Double'
          
          on('command') do
            @text.value = @text.value*2
          end
        }
        
        button {
          grid row: 1, column: column_index += 1, column_weight: 0
          text 'Reset'
          
          on('command') do
            @text.value = VALUE
          end
        }
      }
      
      @text = text {
        grid row: 1, column: 0, row_weight: 1
        wrap 'word'
        value VALUE
      }
    }.open
  end
end

TestText.new.launch

Frankly, there wasn't much difference either way.

I just did this Pull Request implementation out of gut instinct, but it is perhaps a form of premature optimization since I never encountered performance issues with setting the value attribute of Tk::Text. It seems that ActiveTcl does some optimizations behind the scenes so that they are not needed in Ruby.

As a result, I am closing this.