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

Latest spdlog slows down shutdown #42396

Closed
joaomoreno opened this issue Jan 30, 2018 · 7 comments
Closed

Latest spdlog slows down shutdown #42396

joaomoreno opened this issue Jan 30, 2018 · 7 comments
Assignees
Labels
Milestone

Comments

@joaomoreno
Copy link
Member

joaomoreno commented Jan 30, 2018

image

	dispose(): void {
		let then = new Date().getTime();
		this.logger.flush();
		let now = new Date().getTime();
		console.log('FLUSH', now - then);
		then = now;
		this.logger.drop();
		now = new Date().getTime();
		console.log('DROP', now - then);
	}

cc @bpasero

@gabime
Copy link

gabime commented Jan 31, 2018

@joaomoreno no need to do both flush and drop. spdlog does flush when dropped. This should reduce by half the wait time for dispose.

@joaomoreno
Copy link
Member Author

joaomoreno commented Jan 31, 2018

@gabime Hey man! Thanks for checking up on us!

Yeah we found out about that recently, yet didn't adopt it. I'll give that a try. Is there any other thing that comes to mind which would make flush/drop slower from 0.14.0 to 0.16.3?

@gabime
Copy link

gabime commented Jan 31, 2018

@flush in async mode might be slower indeed, since it waits for the queue te be empty(which itself takes up to 500ms even on empty queue)

possible fixes:

  1. I think callng flush is not really needed in your case . If i understand correctly, vscode enables the periodic flush of 2 seconds in async mode anyway (which happens in the bg thread and doesnt block the caller).

  2. I am willing to release an update for spdlog in which flush wont wait for the queue to be empty before returning. It will still be on by default to preserve back compatibly , but a flag or something will configure this.

  3. Use sync mode - it is so fast that a human wont notice any difference anyway. It would also preserve resources, since in async mode, a dedicated thread is created per each logger (but then you need to take care of flushing, since no periodic flush exists in sync mode)

@joaomoreno
Copy link
Member Author

flush in async mode might be slower indeed, since it waits for the queue te be empty(which itself takes up to 500ms even on empty queue)

So, what has changed since 0.14.0? Was flush() not waiting for the queue to be empty, before?

@gabime
Copy link

gabime commented Jan 31, 2018

No, The wait was increased from 200ms to 500ms to reduce cpu usage (gabime/spdlog@b1be7b9#diff-3aed88857f9442521f921ff98e021e33)

@joaomoreno
Copy link
Member Author

Oh I see!

@joaomoreno
Copy link
Member Author

joaomoreno commented Jan 31, 2018

Removed the call to flush. Seems speedy again! I see drop taking anywhere from 25ms up to 439ms. This is fine as it doesn't seem to disturb any quit or reload perception.

Thanks for the help @gabime, no need to release any update.

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants