Ticket #5060 (closed defect: fixed)

Opened 14 months ago

Last modified 14 months ago

ColorPalette extremely slow with IE6

Reported by: jfcunat Owned by: bill
Priority: normal Milestone: 1.0.1
Component: Dijit Version: 1.0
Severity: normal Keywords:
Cc: jeanfrancois.cunat@…

Description

I have some remarks about the ColorPalette? component. It is extremely slow on IE 6 (compared to the dojo 0.4.3 version). I know there are some new features concerning accessiblity but the thing is that if you refresh your browser serveral times, it is getting worse and worse.

dijit/tests/test_ColorPalette.html

Attachments

ColorPalette.js (8.8 kB) - added by jfcunat 14 months ago.
ColorPalette? enhancement for performance issue
5060.patch (2.3 kB) - added by bill 14 months ago.
patch version of changes

Change History

  Changed 14 months ago by jfcunat

  • status changed from new to assigned

Changed 14 months ago by jfcunat

ColorPalette? enhancement for performance issue

Changed 14 months ago by bill

patch version of changes

follow-up: ↓ 3   Changed 14 months ago by bill

  • owner changed from jfcunat to bill
  • status changed from assigned to new

Thanks for the patch. I'll merge this. I assume the real issue was the typematic connects not getting released? (need to check other widgets to see if they have that same problem).

BTW in the future please submit patches as patch files (use your favorite tool to generate the patch file, like tortoise or eclipse or idea). That lets us see what changed.

Was there any reason you moved the color and colorValue declarations outside the loop? I don't see why that would make it faster (although I do see the reason for only having one dojo.Color object).

in reply to: ↑ 2   Changed 14 months ago by jfcunat

Replying to bill:

Thanks for the patch. I'll merge this. I assume the real issue was the typematic connects not getting released? (need to check other widgets to see if they have that same problem). BTW in the future please submit patches as patch files (use your favorite tool to generate the patch file, like tortoise or eclipse or idea). That lets us see what changed. Was there any reason you moved the color and colorValue declarations outside the loop? I don't see why that would make it faster (although I do see the reason for only having one dojo.Color object).

What was really slow was the multiple call do dojo.moduleUrl for the same image ... For the declarations outside the loops I was wondering if we speed up a little bit because we declare them only once instead of several times inside the double loop. But If you say me that it makes no difference, you can let it as they were...

follow-up: ↓ 5   Changed 14 months ago by bill

Hmm, I tried this patch; it didn't seem to make a difference. http://download.dojotoolkit.org/release-1.0.0/dojo-release-1.0.0/dijit/tests/test_ColorPalette.html does take a long time to load (about 5 seconds) but it doesn't seem any faster with this patch, and also it doesn't seem to slow down by multiple loading. Are you seeing something different? I am testing on IE6.

in reply to: ↑ 4 ; follow-up: ↓ 6   Changed 14 months ago by jfcunat

Replying to bill:

Hmm, I tried this patch; it didn't seem to make a difference. http://download.dojotoolkit.org/release-1.0.0/dojo-release-1.0.0/dijit/tests/test_ColorPalette.html does take a long time to load (about 5 seconds) but it doesn't seem any faster with this patch, and also it doesn't seem to slow down by multiple loading. Are you seeing something different? I am testing on IE6.

In fact, I tested my patch with a 0.9 version and I changed it for 1.0. It was really faster in 0.9 than in 1.0 (especially after few refresh). I think I have found a problem in 1.0. We go twice in dojo.addOnUnLoad of _Widget.js file which causes an error at the second time and the _destroyElement is never called, that is why loading is longer and longer in DOjo 1.0 even with my patch. I think the problem is worse than just a problem of ColorPalette? but it is for all templated widgets in IE6 and dojo 1.0.

in reply to: ↑ 5 ; follow-up: ↓ 7   Changed 14 months ago by jfcunat

Replying to jfcunat:

Replying to bill:

Hmm, I tried this patch; it didn't seem to make a difference. http://download.dojotoolkit.org/release-1.0.0/dojo-release-1.0.0/dijit/tests/test_ColorPalette.html does take a long time to load (about 5 seconds) but it doesn't seem any faster with this patch, and also it doesn't seem to slow down by multiple loading. Are you seeing something different? I am testing on IE6.

In fact, I tested my patch with a 0.9 version and I changed it for 1.0. It was really faster in 0.9 than in 1.0 (especially after few refresh). I think I have found a problem in 1.0. We go twice in dojo.addOnUnLoad of _Widget.js file which causes an error at the second time and the _destroyElement is never called, that is why loading is longer and longer in DOjo 1.0 even with my patch. I think the problem is worse than just a problem of ColorPalette? but it is for all templated widgets in IE6 and dojo 1.0.

I am afraid it comes from loader.js at line 253 dojo._loadModule = dojo.require = function(/*String*/moduleName, /*Boolean?*/omitModuleCheck) It makes the dojo.require goes twice is the same module and execute code inside it twice

Before dojo.require was defined below like this. dojo.require= dojo._loadModule;

in reply to: ↑ 6   Changed 14 months ago by jfcunat

Replying to jfcunat:

Replying to jfcunat:

Replying to bill:

Hmm, I tried this patch; it didn't seem to make a difference. http://download.dojotoolkit.org/release-1.0.0/dojo-release-1.0.0/dijit/tests/test_ColorPalette.html does take a long time to load (about 5 seconds) but it doesn't seem any faster with this patch, and also it doesn't seem to slow down by multiple loading. Are you seeing something different? I am testing on IE6.

In fact, I tested my patch with a 0.9 version and I changed it for 1.0. It was really faster in 0.9 than in 1.0 (especially after few refresh). I think I have found a problem in 1.0. We go twice in dojo.addOnUnLoad of _Widget.js file which causes an error at the second time and the _destroyElement is never called, that is why loading is longer and longer in DOjo 1.0 even with my patch. I think the problem is worse than just a problem of ColorPalette? but it is for all templated widgets in IE6 and dojo 1.0.

I am afraid it comes from loader.js at line 253 dojo._loadModule = dojo.require = function(/*String*/moduleName, /*Boolean?*/omitModuleCheck) It makes the dojo.require goes twice is the same module and execute code inside it twice Before dojo.require was defined below like this. dojo.require= dojo._loadModule;

Forget what I've said above -> I found the problem of double loading : it comes from the debugAtAllCosts: true of the test. Remove it and you will see the difference of the ColorPalette? test without or with my patch. It is faster. Check with many reloads on IE 6

  Changed 14 months ago by bill

  • status changed from new to closed
  • resolution set to fixed

(In [11493]) Fixes #5060 on 1.0 branch: ColorPalette? inefficient code and memory leak on IE6. Also makes _Templated.js's dojo.addOnUnload() call itempotent since it executes twice when debugAtAllCosts is true (refs #5091).

Thanks to Jean Cunat (Orange-ftgroup CCLA on file) for patch.

  Changed 14 months ago by bill

(In [11494]) Fixes #5060 on trunk: ColorPalette? inefficient code and memory leak on IE6. Also makes _Templated.js's dojo.addOnUnload() call itempotent since it executes twice when debugAtAllCosts is true (refs #5091).

Thanks to Jean Cunat (Orange-ftgroup CCLA on file) for patch.

Note: See TracTickets for help on using tickets.