Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort Lexographically Ascending: Bad Allocation #10435

Closed
stevemk14ebr opened this issue Aug 23, 2021 · 2 comments
Closed

Sort Lexographically Ascending: Bad Allocation #10435

stevemk14ebr opened this issue Aug 23, 2021 · 2 comments
Assignees
Labels
accepted bug crash issue causing N++ to crash

Comments

@stevemk14ebr
Copy link

stevemk14ebr commented Aug 23, 2021

Description of the Issue

With a sufficiently large text file (1.5 million lines of sha256 hashes, 64 characters each), line operation sort lexographically ascending will 100% of the time fail with a bad allocation warning and then notepad++ will crash with a data loss warning.

Steps to Reproduce the Issue

  1. Open Big file
  2. Sort
  3. Don't Profit

Expected Behavior

Sorting to not crash

Actual Behavior

Crash

Debug Information

Notepad++ v8.1.1 (32-bit)
Build time : Jul 1 2021 - 14:04:28
Path : C:\Program Files (x86)\Notepad++\notepad++.exe
Command Line : "\wsl$\Debian\home\steve\source\repos\Flareup2\utility_scripts\sha256.txt"
Admin mode : OFF
Local Conf mode : OFF
Cloud Config : OFF
OS Name : Windows 10 Enterprise (64-bit)
OS Version : 2009
OS Build : 19043.1165
Current ANSI codepage : 1252
Plugins : ComparePlugin.dll mimeTools.dll NppConverter.dll NppExport.dll NPPJSONViewer.dll NppTextFX.dll

@chcg chcg added bug crash issue causing N++ to crash labels Aug 25, 2021
@chcg
Copy link
Contributor

chcg commented Aug 25, 2021

@felix-22
Is this also happening for the x64 version of N++?
Might be related to 32bit process limit of 2 GB.

@mshingote
Copy link

mshingote commented Dec 25, 2021

Sharing some observations for discussion.

In following file there is one virtual function sort which takes collection of lines as a copy.
...notepad-plus-plus\PowerEditor\src\MISC\Common\Sorters.h:74
virtual std::vector<generic_string> sort(std::vector<generic_string> lines) = 0;

One sorting case from following file
...notepad-plus-plus\PowerEditor\src\ScintillaComponent\ScintillaEditView.cpp

std::vector<generic_string> splitText = stringSplit(text, getEOLString());
...
const std::vector<generic_string> sortedText = pSort->sort(splitText);

Lines are passed as a copy for sorting, seems there is room for memory allocation optimization by referencing existing list of lines.

I may be wrong as I'm not fully aware of end to end flow.

@donho donho self-assigned this Dec 26, 2021
@donho donho added the accepted label Jul 1, 2023
donho added a commit to donho/notepad-plus-plus that referenced this issue Jul 1, 2023
Use reference instead of copy.
Also improve lines sorting performance slightly: Sorting a 200 MB text file takes 13.71 seconds instead of 14.63 seconds.

Fix  notepad-plus-plus#10435
@donho donho closed this as completed in 9e24ec5 Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted bug crash issue causing N++ to crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@donho @stevemk14ebr @mshingote @chcg and others