json_BufferAppend replaced with clsStringAppend #82
+308
−141
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replaced calls to json_BufferAppend with simple class module clsStringAppend. Code is faster (x5), simpler and should work on Mac as well as Windows, though I have not tested that...
|
Hi @PGS62 thanks for the PR! Sorry it's taken me so long to respond. This is really attractive to me, don't like the dependence on Windows API calls and manually working with memory is always a little nerve-wracking. I'll make some tweaks and merge this soon. 👍 |
|
Ah, glad you like the look of my change. It was my first pull request on GitHub! |
|
@PGS62 , good job, you saved me ! |
|
Just wanted to add my thanks -- I endured some painful performance for a week until I had time to sit down to debug it and was delighted to see a fix here. |
|
@PGS62, thank you for this update |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaced calls to json_BufferAppend with simple class module clsStringAppend. Code is faster (x5), and is simpler (no Windows API calls), and simpler to call.
I have not tested my changes on Mac, but existing method json_BufferAppend does "naive" string appending on Mac and so will presumably have poor performance. In contrast clsStringAppend ought to have good performance.
Downside of my change is that users of the code have to import a class module (clsStringAppend) as well as the module JsonConverter. They should be OK with this surely? Though it will make the nifty training video a bit out-of-date. :-(
The module modTest1 is a test harness for json_BufferAppend versus clsStringAppend, and modTest2 is a really simple check that the changes I've made work. Neither of those modules need to be in the project.