Output each char individually, avoid cost of appending to static string#138
Open
joelself wants to merge 2 commits intojcjohnson:masterfrom
Open
Output each char individually, avoid cost of appending to static string#138joelself wants to merge 2 commits intojcjohnson:masterfrom
joelself wants to merge 2 commits intojcjohnson:masterfrom
Conversation
|
One thing I see your commit does not address is printing the start text to the output. I would love to merge this to my fork if you fix that. |
Write the start text at the beginning of the output.
Author
|
Updated to write out the start text before anything else (other than the verbose stuff). |
|
Thanks. I'm going to work on smoothly integrating it with a couple of other PRs related to output then integrate it. |
Author
|
Cool. I'm also working on more performance improvements, but I'm finding Lua tooling to be somewhat non-existent. |
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
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.
This loop in
LanguageModel.luais a killer for performance with strings greater than about 100,000 characters. Like a lot of languages strings in Lua are static so appending to them requires reallocating a new chunk of memory, copying all of the previous string into the new memory and adding the new character to the end. Of course they try and mitigate it by doubling the size of the string array each reallocation, but while this performs well in theory, in practice it still kills run time. Here's some perf numbers for various size strings running the original code (model_type: lstm, rnn_size: 256, layers: 3, temp: 0.9):
For some reason it runs out of memory at 1,000,000 characters so the 15.6x slowdown compared to 100,000 characters is probably smaller than if it actually finished.
Here's the perf numbers for this pull request which outputs each character as soon as it is generated, avoiding repeated appends to a large string:
With smaller strings the start-up time dominates and there's not much difference. At around 100,000 characters the new code just starts to show real improvements. At 800,000 characters the old code is 11.3x as slow as generating 100,000 characters and it only gets worse as numbers get bigger. The new code is 7.7x as slow, less than the expected 8x slowdown.
There's ways to keep the string in memory and only output it at the end effeciently. But outputting the characters immediately is helpful when first learning
torch-rnn/torchas well as when tuning thetemperatureparameter on a newly trained RNN.