#5060 closed defect (fixed)
ColorPalette extremely slow with IE6
Reported by: | jfcunat | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.0.1 |
Component: | Dijit | Version: | 1.0 |
Keywords: | Cc: | [email protected]… | |
Blocked By: | Blocking: |
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 (2)
Change History (11)
comment:1 Changed 13 years ago by
Status: | new → assigned |
---|
Changed 13 years ago by
Attachment: | ColorPalette.js added |
---|
comment:2 follow-up: 3 Changed 13 years ago by
Owner: | changed from jfcunat to bill |
---|---|
Status: | assigned → 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).
comment:3 Changed 13 years ago by
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...
comment:4 follow-up: 5 Changed 13 years ago by
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.
comment:5 follow-up: 6 Changed 13 years ago by
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.
comment:6 follow-up: 7 Changed 13 years ago by
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;
comment:7 Changed 13 years ago by
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
comment:8 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(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.
comment:9 Changed 13 years ago by
(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.
ColorPalette? enhancement for performance issue