#474 Some code corrections and refinements

Next_release
closed
nobody
None
7
2013-07-03
2013-05-04
FLS
No

Within this patch, some code corrections and refinements are gathered.
Patches are based on SVN 1036.

1.) Unused module "UniConversion"
The routines in "UniConversion.cpp, UniConversion.h" are not used. The modules can be deleted from the project.
Furthermore, files with the same names are present in scintilla!
Just delete "#include UniConversion.h" within FindReplaceDlg.cpp

2.) Delete bin\npp.pdb from Notepad++ SVN repository
That file seems unnecessary for the SVN repository and annoys always by signalling differences where no are present.

3.) Loading Time resolution too coarse -> new module ElapseTimer
The resolution of the displayed duration to load Notepad++ (e.g. command line parameter -loadingTime) is only full seconds and NOT as described in the Notepad++ documentation with a resolution of 0.01 seconds.
This is because the "time()" function is used, which only returns an integer giving the time in seconds.
This patch includes an additional module "ElapseTimer", which uses the more accurate PerformanceCounter.
This module can also be used to measure time differences down to micro-seconds or even nano-seconds allowing measurement of code execution times and compare it to alternative codings. Just include ElapseTimer.h for debugging purposes.
By the way, the code is stolen from Scintilla PlatWin.cxx because I couldn't figure out, how to access that internal Scintilla function.
(see Patch NppPatch_6.3.2_ElapseTimer.patch)

4.) Speed up getCurrentFoldStates()
The newly provided code using SCI_CONTRACTEDFOLDNEXT is usually 10%-50% faster than checking each line of the document!!
You can easily prove it using the ElapseTimer by including the ElapseTimer.h file for debugging and time measurement purposes.
(see Patch NppPatch_6.3.2_SpeedUp-GetFoldingStates.patch)

1 Attachments

Discussion

1 2 > >> (Page 1 of 2)
  • Isn't npp.pdb needed if a user sends in a crash dump and you want to look at it in Visual Studio? I don't think it's enough to regenerate the PDB file since then it won't match the executable which Don releases. (I remember reading a very detailed article about this written by someone on the VC++ team. Apparently the compiler/linker isn't guaranteed to be deterministic, so if you rebuild the project twice it might not generate the same executable. At my job we zip and save several gigabytes worth of PDB files for this reason every time we release a new build of the project.)

    While we're at it, I would like to see notepadPlus.vcproj removed and replaced with the converted .vcxproj file so everyone doesn't have to run the conversion themselves all the time.

     
  • Don HO
    Don HO
    2013-05-18

    @FLS:
    Thank you for the patches.
    I did test NppPatch_6.3.2_SpeedUp-GetFoldingStates.patch, however I cannot reproduce the performance of 10%-50% faster that you described.

    Could you give me the instruction to check the difference?

    Don

     
  • Don HO
    Don HO
    2013-05-18

    Yes Andreas is right about the npp.pdb, so it should be kept.

    OTOH, what is the advantage to use vcxproj instead of vcproj ?

    Don

     
  • Well, I don't know which version of VC++ you use but 2010 refuses to open vcproj files. It insists on converting them to vcxproj and then building that. It's only a small matter of course, but still a bit annoying.

    It also leads to this problem: I open the vcproj and convert it to vcxproj. Then you add a source file to be built to the vcproj. I get the latest trunk version which updates the vcproj, but not the vcxproj. The next time I try to build I'll get an error and have to regenerate the vcxproj.

     
  • FLS
    FLS
    2013-05-18

    @Andreas:
    Thank you for the information about the npp.pdb file. I didn’t know that.

    For the issue about vcxproj vs. vcproj you are right for VC2010 but what are about guys like me, who are still using VC2008? We would have the same trouble. Even worse, because as I know, VC2008 refuses to convert VC2010 project files!

    @Don:
    I just measured the two different implementations within the same function using PerformanceCounter (see Patch NppPatch_6.3.2_ElapseTimer.patch). So it is not a performance of the complete Notepad++ but only the performance of the function getCurrentFoldStates().

    For your convenience, I attach a patch with the debugging / time measuring code below.

    FLS

     
  • Yeah, it's true that it would be problematic for those using VC2008.

     
  • Don HO
    Don HO
    2013-05-18

    @FLS:
    I do understand there's no difference on startup of Notepad++ since getCurrentFoldStates() is not called while loading.
    What I need is the instructions to reproduce the different performance (10-50% that you mentioned about). Without the instructions, I don't see the reason to accept NppPatch_6.3.2_SpeedUp-GetFoldingStates.patch.

    @Andreas:
    Notepad++ releases is still built by VS2005.

    Don

     
  • FLS
    FLS
    2013-05-19

    @Don
    Maybe there is a misunderstanding. The influence on Notepad++ startup is not measurable. The code change only concerns the execution time of the function getCurrentFoldStates(), which is called when a file is opened or closed or every time a new file tab (buffer) is activated.

    On my PC, some measured differences for files with folding (HTML, XML, CPP) between the old and the new code are:

    Old Code         New Code       Factor old/new
     0.20 ms          0.0008 ms         250
     0.20 ms          0.0010 ms         200
     0.15 ms          0.0500 ms           3
     0.06 ms          0.0007 ms          85
     0.15 ms          0.0800 ms           1.8
     0.02 ms          0.0007 ms          28
    

    The procedure to measure that is:

    • Apply patch NppPatch_6.3.2_ElapseTimer.patch, which brings the PerformanceCounter function into the code. Maybe the two files ElapseTimer.cpp and .h have to be added manually to the VisualStudio project.
    • Apply patch NppPatch_6.3.2_TimeMeasure_SpeedUp-GetFoldingStates.patch which includes a debugging version of ScintillaEditView.cpp with the old and the new code within getCurrentFoldStates(), so both codes are executed one after the other. Both execution times are measured and assigned to debugging variables elapsedTime1 and elapsedTime2. The ratio between the two execution times is given in factor1 (new/old) and factor2 (old/new).
    • Compile and set a breakpoint at the end of function getCurrentFoldStates()
    • Run some tests by opening and closing different HTML, XML, CPP files. Each time the debugger stops at the breakpoint you can observe the values of the debugging variables.

    Even when the absolute performance optimization is not so dramatic, I considered it worthwhile to provide that patch because of performance improvements of factor 3 to 200 within getCurrentFoldStates(). IMHO a lot of small improvements boost at the end the whole application.

    FLS

     
  • Don HO
    Don HO
    2013-06-09

    • status: open --> accepted
    • Priority: 5 --> 7
     
1 2 > >> (Page 1 of 2)