Speed up Javascript Loop ??


M

Mel Smith

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;
}
 
Ad

Advertisements

D

Denis McMahon

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
 
D

Denis McMahon

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
 
T

typo

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
 
M

Mel Smith

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
 
S

SAM

Le 14/08/10 20:09, typo a écrit :
Hi Mel,

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 ... :-(
 
Ad

Advertisements

D

David Mark

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).
 
D

Dr J R Stockton

Sat said:
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.
 
Ad

Advertisements

R

RobG

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.


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.
 

Top