https://issues.apache.org/bugzilla/show_bug.cgi?id=49165
--- Comment #6 from Alexander Shirkov <sgdr...@gmail.com> 2010-07-22 22:38:38 EDT --- (In reply to comment #5) > 1. It uses a mix of tabs and spaces for indenting. Only spaces should be used. Done. > 2. Log messages should use the StringManager to provide i18n support. Done (only English string was added). BTW there are others Strings in class used in exception. I haven't touched them, because they're not related to this bug. Please let me know if this should be done. > 3. Setting currentDate in the AccessDateStruct serves no purpose Removed initialization in variable declaration. Leaved as is in constructor: otherwise NPE will be thrown on first AccessDateStruct access in startInternal(). > 4. Same for currentDateString Removed > 5. DateAndTimeElementVolumeTest can probably be added to the existing > Benchmarks test. Done. > Out of curiosity, if you remove the code that manages the only creating the > date once a second entirely and use a date format that does not include > millis, > what is the performance like? I'm wondering if we can just remove that code > entirely. Without millis chunks we have small performance drop on same %t pattern: 5 thr x 1000000 iter. newValve 7772ms 5 thr x 1000000 iter. newValveNoChunks 8483ms 5 thr x 1000000 iter. newValveWithMillis 8630ms 5 thr x 2000000 iter. newValve 14857ms 5 thr x 2000000 iter. newValveNoChunks 15633ms 5 thr x 2000000 iter. newValveWithMillis 19687ms 5 thr x 3000000 iter. newValve 22171ms 5 thr x 3000000 iter. newValveNoChunks 23433ms 5 thr x 3000000 iter. newValveWithMillis 26765ms 5 thr x 4000000 iter. newValve 29615ms 5 thr x 4000000 iter. newValveNoChunks 31525ms 5 thr x 4000000 iter. newValveWithMillis 35067ms 5 thr x 5000000 iter. newValve 36889ms 5 thr x 5000000 iter. newValveNoChunks 39176ms 5 thr x 5000000 iter. newValveWithMillis 43527ms Looks like performance graphs are linear for all cases. I think if we remove chunks, this could possible cause slight load increase on GC under heavy load (new Date instances will keep creating). Test was done under MacOS X 10.6. Please let me know your decision: are we going to go with this or leave it as is. BTW there will be error up to 1000 mills in output, because we're creating date once in 1000 millis. Note: volume benchmark is writing output in real file to $CATALINA_HOME/logs and shouldn't be included in automatic test pack (it's slow and it's it's generating REALLY big output file, so CI server will quickly run out of disk space). > If you can update the patch, I'll take another look. With the issues above > fixed it should be ready to apply. Will provide patch file after your decision. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
