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

Improve performance of spliceChangesIntoString #311

Closed
wants to merge 1 commit into from

Conversation

ABuffSeagull
Copy link
Contributor

The original algorithm caused a lot of memory pressure, especially in large files.
For a 91 KB file at my job, this change speeds it up from 101509ms to 1095ms.

@thecrypticace
Copy link
Contributor

I opened a PR with these changes, some additional optimizations, tests, and benchmarks: #312

I would've edited this PR but GH wasn't allowing me to for some reason (it says I can edit but it wasn't letting me 🤷 )

Thank you for figuring this one out. Closing in favor of that PR (you're still credited btw!)

@shyakadavis
Copy link

I would've edited this PR but GH wasn't allowing me to for some reason (it says I can edit but it wasn't letting me 🤷 )

Hi,

I don't mean to spam, but I've seen some comments from huntabyte, saying that the issue here is caused by the fact that one opens a P.R from the main branch of a fork, targetting main on, well, the main repo.

A simple solution was always to open the P.R from a different branch, then maintainers can commit directly to it.

Cheers.

P.S. I found myself here after digging through the new release(s) (v0.6.8 & v0.6.7) which were breaking after renovate bot tried updating. Glad it's working now. 🙂

Cc: @thecrypticace @ABuffSeagull

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.

3 participants