Skip to content

perf: optimize countWords function for large payloads#585

Closed
Ashura707 wants to merge 4 commits intolingodotdev:mainfrom
Ashura707:optimize-countWordsInRecord_
Closed

perf: optimize countWords function for large payloads#585
Ashura707 wants to merge 4 commits intolingodotdev:mainfrom
Ashura707:optimize-countWordsInRecord_

Conversation

@Ashura707
Copy link
Copy Markdown

Fix:#569
Before:
Screenshot 2025-03-26 100720

After:
Screenshot 2025-03-26 100547

->Removed recursion based approach and made it iterative to process it.Added test cases,
->Optimized processing speed of large payloads increasing the performance

@Ashura707
Copy link
Copy Markdown
Author

@maxprilutskiy could the lingo dev team check this and let me know

Comment on lines +1 to +2
---
---
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a changeset if we want to release the change.

Comment thread packages/sdk/src/index.ts Outdated
const stack: any[] = [payload];
let totalWordCount = 0;

while (stack.length > 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this an optimization? If I am reading this right, the code is executed the same number of times as before. This is a micro-optimization at best (if while is faster than recursion), however I find this new code harder to read than before.

I'd like a second pair of eyes on this, @maxprilutskiy @7hacker please have a look.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathio I can come up with a better optimization for this making it more readable.Should I make a new pr or add the changes in this branch?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I raised my concerns in the issue. Lets continue the conversation there.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New Optimizations
Screenshot 2025-03-27 084005
->Improved execution time for large and deeply nested payloads.
->Lower memory consumption by eliminating redundant allocations.
->Removed Recursion because it might cause stack overflow.

@mathio
Copy link
Copy Markdown
Contributor

mathio commented Mar 27, 2025

@Ashura707 we decided to take a different direction here. Good job optimizing the performance, however we prefer simplicity and better maintainability over maximum performance here.

Sorry for not raising it internally before we opened an issue.

@mathio mathio closed this Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants