Skip to content

Serve rebuilds for every file individually when a lot of files change #679

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

Closed
azerupi opened this issue Apr 27, 2018 · 5 comments · Fixed by #870
Closed

Serve rebuilds for every file individually when a lot of files change #679

azerupi opened this issue Apr 27, 2018 · 5 comments · Fixed by #870
Labels
A-Performance Area: Zoom Zoom C-papercut Category: A small usability bug

Comments

@azerupi
Copy link
Contributor

azerupi commented Apr 27, 2018

When a lot of source files change and we were running mdbook serve, it will rebuild the book for each file that changed. This is particularly painful when using git and changing branches. During those rebuilds, the served book is broken.

Simply quitting mdbook and relaunching the command solves the problem, but it is an annoying paper-cut.

@azerupi azerupi added A-Performance Area: Zoom Zoom C-papercut Category: A small usability bug labels Apr 27, 2018
@Michael-F-Bryan
Copy link
Contributor

Michael-F-Bryan commented May 5, 2018

I just noticed this myself when I added a "todo: write this" note to all the unwritten chapters in something I'm working on. It's a real pain when the mdbook watch/mdbook serve tries to rebuild for every updated file when really it should only do the update once.

The actual culprit is our trigger_on_change() function in src/bin/watch.rs because it'll re-run the function for every change notification, but I'm not 100% sure how we can deal with that. We'd need some sort of "debounce" mechanism to collate a bunch of notifications arriving at the same time into a single "stuff changed" message.

What are your thoughts on something like this?

loop {
  let _ = rx.recv();  // block until the first change comes in
  sleep(50ms);  
  let _ = rx.try_iter().count(); // collect all other notifications received while sleeping
  rebuild();
}

@azerupi
Copy link
Contributor Author

azerupi commented May 5, 2018

That could work. Notify seems to send an event for every file changed, even within the 1 second debounce time we give it. So flushing the iterator should do the trick. However, we should decently document/comment this portion of code because it is not obvious what it does. 😉

I also think that last() could be used instead of count on the iterator if we do not use the number of changes detected in logs or other things. Both consume the iterator, so I think it should work the same but I find last() more obvious in its intent.

@Michael-F-Bryan
Copy link
Contributor

Both consume the iterator, so I think it should work the same but I find last() more obvious in its intent.

Agreed. Although it could also be turned into some sort of collect() so we can log which files were updated and how... Or is that overkill?

@azerupi
Copy link
Contributor Author

azerupi commented May 5, 2018

Yes, absolutely! It comes down to what you want to log.

@cauebs
Copy link
Contributor

cauebs commented Feb 2, 2019

I forgot to mention, but I've opened #870 to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Performance Area: Zoom Zoom C-papercut Category: A small usability bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants