Speed up Javascript Loop ??

Discussion in 'Javascript' started by Mel Smith, Aug 14, 2010.

  1. Mel Smith

    Mel Smith Guest

    Hi:

    As my script below explains, I'm trying to speed up a search of a long
    table (5600 rows x 5 columns). This table's columns are filled with text
    (except with column 0 which contains a button.

    Currently, my search code takes about 3 seconds to search thru 1, 2, or
    3 (column 0 is the 'button' column) hiding all rows that don't contain the
    searched-for text. After the search is complete, and the user wishes to see
    the complete table again, this 'un-hiding' takes about 5 seconds.

    But these times are for Chrome, IE7 and Safari. For FireFox, the times
    are 3 times longer (i.e., worse) !!

    I have two major questions:

    What is there about the code below that makes FF perform so poorly ?
    How can I change the code to make them *all* perform faster (even at
    the expense of safety) ?

    Thank you for any advice offered !

    (and No, I won't show the complete web page containing this code for
    another month or so -- until it is ready for 'prime time'. )
    --
    Mel Smith

    ******************************************
    /* This next function is called to search for
    a fragment of text in a specific column of a long table
    (approx 5600 rows of 5 columns). The values in each of
    the right-most columns are pure text.
    This 'searching' function is fastest in Chrome, fast in
    Safari, nearly as fast in IE7, and *very slow* in FF.
    In fact, FF is only 1/3 as fast as Chrome and less than
    1/2 as fast as IE7.

    Question:
    What can be changed in the following code to speed it
    up ? btw, Because I know (and control) the exact structure of
    the table, I have removed all error checking from the code.
    What else can I do for speed only please ?
    */
    /*-------------------------------*/
    function tablesearch() {
    var i,tr,tds,td,skipcount,searchcol ;
    var term = SEARCH_INPUT.value.toLowerCase(); / user typed in this text
    // vrbls rows[] and nrows are both global vrbls
    // rows.length == nrows == 5564

    // skip first few rows
    skipCount= SKIP_ROW_COUNT ; // use a local vrbl -- this is 2 for now
    searchcol = SEARCH_COLUMN ; // ditto (col 0==button) -- use col 1, 2, or
    3 for now, 4 later

    for (i = skipCount; i < nrows; i++) {
    tr = rows; // access an individual row here
    tds = tr.getElementsByTagName("td"); // maybe something could be done
    here ??
    td = tds[searchcol] ; // now we have the column to be searched
    tr.style.display = (term == "" || doesNodeContain(td.firstChild, term)) ?
    "" : "none"; // don't really know what I'm doing here
    }
    }

    function doesNodeContain(node, term) {
    if (node.nodeValue.toLowerCase().indexOf(term) >= 0) {
    return true;
    }
    return false;
    }
    Mel Smith, Aug 14, 2010
    #1
    1. Advertising

  2. On 14/08/10 17:03, Mel Smith wrote:

    > As my script below explains, I'm trying to speed up a search of a long
    > table (5600 rows x 5 columns). This table's columns are filled with text
    > (except with column 0 which contains a button.


    Untested:

    // search specified column of table for text containing a regex
    // skip a number of header rows
    // hide all rows where the text is not found in the column,
    // show all rows where it is
    function showTableRowsContaining(tableId,colNum,searchFor,skipRows)
    {
    var i, theTable, theRow;
    theTable = document.getElementById(tableId);
    for (i = skipRows; i < theTable.rows.length; i++)
    {
    theRow = theTable.rows;
    if (theRow.cells[colNum].innerHTML.match(searchFor))
    theRow.style.visibility = "visible";
    else theRow.style.visibility = "collapse";
    }
    }

    Rgds

    Denis McMahon
    Denis McMahon, Aug 14, 2010
    #2
    1. Advertising

  3. On 14/08/10 18:16, Denis McMahon wrote:
    > On 14/08/10 17:03, Mel Smith wrote:
    >
    >> As my script below explains, I'm trying to speed up a search of a long
    >> table (5600 rows x 5 columns). This table's columns are filled with text
    >> (except with column 0 which contains a button.

    >
    > Untested:
    >
    > // search specified column of table for text containing a regex
    > // skip a number of header rows
    > // hide all rows where the text is not found in the column,
    > // show all rows where it is
    > function showTableRowsContaining(tableId,colNum,searchFor,skipRows)
    > {
    > var i, theTable, theRow;
    > theTable = document.getElementById(tableId);
    > for (i = skipRows; i < theTable.rows.length; i++)
    > {
    > theRow = theTable.rows;
    > if (theRow.cells[colNum].innerHTML.match(searchFor))
    > theRow.style.visibility = "visible";
    > else theRow.style.visibility = "collapse";
    > }
    > }


    Hmm, having decided to test this, finding a method of hiding and
    displaying complete rows that works in ff and ie isn't easy.

    Rgds

    Denis McMahon
    Denis McMahon, Aug 14, 2010
    #3
  4. Mel Smith

    typo Guest

    Hi Mel,

    >    tds = tr.getElementsByTagName("td");

    this row could be placed outside (before) the loop inside function
    tablesearch, however minimal effect to expect.

    What about 2 tables? One complete table, to hide/unhide, and a second
    table to show the search results.
    How is the table information delivered? I assume, it is prebuilt html.
    What about getting information as array (static or via JSON) and build
    the table dynamically?
    Best would be to have the columns presorted for efficient binary
    search.
    Your code mixes time consuming linear search and dom-manipulation on
    many rows.
    Try to separate to evaluate time consumption.

    best regards
    Roland Frank

    related subject: http://www.en.publicsql.org/
    http://bu4.taipudex.com
    typo, Aug 14, 2010
    #4
  5. Mel Smith

    Mel Smith Guest

    Hi Denis & Type:

    I'll be puzzling and working over your responss this weekend.

    Now its drinking and watching golf on TV time :)

    Thanks for the suggestions !

    -Mel
    Mel Smith, Aug 14, 2010
    #5
  6. Mel Smith

    SAM Guest

    Le 14/08/10 20:09, typo a écrit :
    > Hi Mel,
    >
    >> tds = tr.getElementsByTagName("td");

    > this row could be placed outside (before) the loop inside function
    > tablesearch, however minimal effect to expect.
    >
    > What about 2 tables? One complete table, to hide/unhide, and a second
    > table to show the search results.
    > How is the table information delivered? I assume, it is prebuilt html.
    > What about getting information as array (static or via JSON) and build
    > the table dynamically?
    > Best would be to have the columns presorted for efficient binary
    > search.
    > Your code mixes time consuming linear search and dom-manipulation on
    > many rows.
    > Try to separate to evaluate time consumption.


    that could be a good idea
    as I tried by my side : <http://cjoint.com/?iovcE6K2s3>
    where we have Chrome or Safari.5 doing the job immediately while
    Firefox.3.6 spend a lot of time to decide to display what what wanted
    (the functions seem to run quite fast)
    I haven't FireBug installed in my Chrome

    Not tested IE (I haven't it)
    but IE was more again lazy than Fx with tables ... so ... :-(

    > best regards
    > Roland Frank



    --
    Stéphane Moriaux avec/with iMac-intel
    SAM, Aug 14, 2010
    #6
  7. Mel Smith

    SAM Guest

    Le 14/08/10 21:06, Mel Smith a écrit :
    > Hi Denis& Type:
    >
    > I'll be puzzling and working over your responss this weekend.
    >
    > Now its drinking and watching golf on TV time :)
    >
    > Thanks for the suggestions !


    The suggestion of typo is not so bad at all
    <http://cjoint.com/data/ioxEqWIoGz_table_find.htm>
    and better :
    <http://cjoint.com/data/ioxNssSubF_table_find.htm>


    --
    Stéphane Moriaux avec/with iMac-intel
    SAM, Aug 14, 2010
    #7
  8. Mel Smith

    David Mark Guest

    On Aug 14, 1:16 pm, Denis McMahon <>
    wrote:
    > On 14/08/10 17:03, Mel Smith wrote:
    >
    > >     As my script below explains, I'm trying to speed up a search ofa long
    > > table (5600 rows x 5 columns). This table's columns are filled with text
    > > (except with column 0 which contains a button.

    >
    > Untested:
    >
    > // search specified column of table for text containing a regex
    > // skip a number of header rows
    > // hide all rows where the text is not found in the column,
    > // show all rows where it is
    > function showTableRowsContaining(tableId,colNum,searchFor,skipRows)
    > {
    >   var i, theTable, theRow;
    >   theTable = document.getElementById(tableId);
    >   for (i = skipRows; i < theTable.rows.length; i++)


    That's not going to be very fast as it will haev to resolve the rows
    and length properties on each iteration.

    var l = theTable.rows.length;
    for (i = skipRows; i < l; i++)

    >   {
    >     theRow = theTable.rows;
    >     if (theRow.cells[colNum].innerHTML.match(searchFor))


    Don't match against the markup, which contains tag names, attributes,
    etc. You will need to get the text content. Something like:-

    var theCell = theRow.cells[colNum];
    var theText = theCell.textContent || theCell.innerText || '';

    And don't use the string match method for this either as it will be
    slower than using the test method of the RegExp:-

    if (searchFor.test(theText)) {

    You can consolidate those last two lines to eliminate the variable.

    > theRow.style.visibility = "visible";
    >     else theRow.style.visibility = "collapse";


    That won't do anything in IE < 8. You should use the display style,
    but it requires some feature detection to determine whether "block" or
    "table-row" should be used to show the rows. In any event, use "none"
    to collapse them.

    Determine the appropriate style with a one-off that appends a single-
    row table to the body, checks the "computed" style of the row and then
    removes the table. Quotes indicate you must use the currentStyle
    property in IE as it does not support computed styles. Then store the
    name of the style for use in your loop. Do *not* create a wrapper
    function to show/hide the rows as it will slow things down
    considerably (exiting an execution context gives browsers a chance to
    reflow/repaint).
    David Mark, Aug 14, 2010
    #8
  9. In comp.lang.javascript message <>, Sat,
    14 Aug 2010 10:03:02, Mel Smith <> posted:

    > Currently, my search code takes about 3 seconds to search thru 1, 2, or
    >3 (column 0 is the 'button' column) hiding all rows that don't contain the
    >searched-for text. After the search is complete, and the user wishes to see
    >the complete table again, this 'un-hiding' takes about 5 seconds.


    You *might* do better to hide the ostensible table completely (display
    none) and show a newly-constructed table containing only what has been
    found.


    > But these times are for Chrome, IE7 and Safari. For FireFox, the times
    >are 3 times longer (i.e., worse) !!
    >
    > I have two major questions:
    >
    > What is there about the code below that makes FF perform so poorly ?


    There is a considerable variation between browsers in general JavaScript
    speed; there may be nothing specific in your code that slows FireFox.

    > How can I change the code to make them *all* perform faster (even at
    >the expense of safety) ?


    Rather than scanning the DOM structure, might it not be better to
    maintain the data in an array of arrays and search that? Having found
    the position, you can then alter the display as needed. Try ay least to
    avoid calling toLowerCase repeatedly within a search if it can be
    avoided. It could be faster to match with a RegExp, since that has
    built-in case-handling.

    >Thank you for any advice offered !



    Your tds = tr.getElementsByTagName("td") may be time-consuming. If
    searches are done repeatedly, and the set of elements does not change,
    you can cache the tds values, as in
    <URL:http://www.merlyn.demon.co.uk/js-misc0.htm#Cash>.

    Do not put anything other than a sig under a sig separator.


    >function doesNodeContain(node, term) {
    > if (node.nodeValue.toLowerCase().indexOf(term) >= 0) {
    > return true;
    > }
    > return false;
    >}


    If you don't know enough to be able to write that as

    function doesNodeContain(node, term) {
    return node.nodeValue.toLowerCase().indexOf(term) >= 0 }

    then perhaps your present task is too much for your present stage of
    learning.

    --
    (c) John Stockton, nr London, UK. ?@merlyn.demon.co.uk Turnpike v6.05.
    Web <URL:http://www.merlyn.demon.co.uk/> - w. FAQish topics, links, acronyms
    PAS EXE etc : <URL:http://www.merlyn.demon.co.uk/programs/> - see 00index.htm
    Dates - miscdate.htm estrdate.htm js-dates.htm pas-time.htm critdate.htm etc.
    Dr J R Stockton, Aug 15, 2010
    #9
  10. Mel Smith

    RobG Guest

    On Aug 15, 8:04 am, David Mark <> wrote:
    > On Aug 14, 1:16 pm, Denis McMahon <>
    > wrote:
    > > On 14/08/10 17:03, Mel Smith wrote:
    > > >     As my script below explains, I'm trying to speed up a search of a long
    > > > table (5600 rows x 5 columns). This table's columns are filled with text
    > > > (except with column 0 which contains a button.

    >
    > > Untested:

    >
    > > // search specified column of table for text containing a regex
    > > // skip a number of header rows
    > > // hide all rows where the text is not found in the column,
    > > // show all rows where it is
    > > function showTableRowsContaining(tableId,colNum,searchFor,skipRows)
    > > {
    > >   var i, theTable, theRow;
    > >   theTable = document.getElementById(tableId);
    > >   for (i = skipRows; i < theTable.rows.length; i++)

    >
    > That's not going to be very fast as it will haev to resolve the rows
    > and length properties on each iteration.
    >
    > var l = theTable.rows.length;
    > for (i = skipRows; i < l; i++)


    Alternatively (might need to decrement skipRows by 1 or use >=):

    for (var i = theTable.rows.length; i > skipRows; i--) {
    ...
    }

    [...]

    > And don't use the string match method for this either as it will be
    > slower than using the test method of the RegExp:-
    >
    > if (searchFor.test(theText)) {


    For the OP, presuming the text to search for is in the searchFor
    variable:

    searchFor = new RegExp(searchFor,'i');

    will match case insensitive, saves toLowerCase() call.


    > You can consolidate those last two lines to eliminate the variable.
    >
    > > theRow.style.visibility = "visible";
    > >     else theRow.style.visibility = "collapse";

    >
    > That won't do anything in IE < 8.  You should use the display style,
    > but it requires some feature detection to determine whether "block" or
    > "table-row" should be used to show the rows.  In any event, use "none"
    > to collapse them.


    Easier to just swap the display property between 'none' to hide and
    '' (empty string) to display them.


    --
    Rob
    RobG, Aug 16, 2010
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Ham

    I need speed Mr .Net....speed

    Ham, Oct 28, 2004, in forum: ASP .Net
    Replies:
    6
    Views:
    2,336
    Antony Baula
    Oct 29, 2004
  2. efiedler
    Replies:
    1
    Views:
    2,030
    Tim Ward
    Oct 9, 2003
  3. Replies:
    2
    Views:
    2,286
    Howard
    Apr 28, 2004
  4. Replies:
    2
    Views:
    332
    Christopher Benson-Manica
    Apr 28, 2004
  5. Isaac Won
    Replies:
    9
    Views:
    365
    Ulrich Eckhardt
    Mar 4, 2013
Loading...

Share This Page