[NFC] Use quick_exit to skip global cleanup #8212

kripken · 2026-01-16T00:18:21Z

We have global stores for interned strings and wasm types, and cleaning those
up takes a lot of time. This PR skips them, which makes roundtripping a large
Dart binary 10% faster.

Diff without whitespace is smaller - some code is now indented so that it
runs dtors before main exits.

sbc100 · 2026-01-16T00:33:26Z

src/support/file.cpp

+
+void wasm::flush_and_quick_exit() {
+  std::cout << std::flush;
+  std::cerr << std::flush;


You might want to add fflush(NULL) here, either in addtion or insteadof the above two lines.

If the stream argument is NULL, fflush() flushes all open output streams.

I am not an expert on this, but from the docs I think that is not needed, if all the C++ ostreams are closed? Closing them (e.g. by their destructor) should flush them, so only the standard output streams need special handling?

Sure but fflush(NULL) is fewer lines :)

Or is there some difference between flushing C++ ostreams and C fflush?

Sure but fflush(NULL) is fewer lines :)

But I think that C command flushes the file handles themselves, while the C++ command also flushes anything buffered at the C++ level..? That is, C++ has buffering in ostream, iirc - am I wrong?

Fair enough, I guess you never know if C++ is doing anything here... but i doubt its doing more buffering on top of what C is doing.

I added fflush(NULL). From the docs I am not certain it is not needed, and it seems safe/harmless.

dschuff

... GH won't let me submit a "comment" review with this field blank. Is that new?

dschuff · 2026-01-16T00:33:17Z

src/support/file.cpp

 }
+
+void wasm::flush_and_quick_exit() {
+  std::cout << std::flush;


You could use std::at_quick_exit to register a function to explicitly flush the open descriptors. Can that be done for the output file too, or is that already guaranteed by some other means?

(the advantage of this would be that you could just use std::quick_exit(0) directly at the current flush_and_quick_exit callsites)

I think the output files should be flushed by their destructors? (this PR ensures those destructors run before exit)

Interesting about at_quick_exit - that could work, though I don't have a preference myself (doesn't seem like a problem to wrap quick_exit, in fact it might be nice for debugging) - wdyt?

I don't have a strong opinion, it doesn't make that much difference in the end, you either have this wrapper, or a callback. You wouldn't have to worry about plumbing the error code through or not, but you are right that the control is conceptually simpler without the callback.

Yeah, I think it's simpler without the callback - plumbing through the code, which I just added, is easy enough.

sbc100 · 2026-01-16T00:35:02Z

src/tools/wasm2js.cpp

    std::cerr << "done." << std::endl;
  }
+
+  flush_and_quick_exit();


Maybe flush_and_quick_exit(0) here? i.e. take the exit code as an argument.

Otherwise you should call the function flush_and_quick_exit_with_zero :)

I was thinking that for errors we don't really care about a quick exit anyhow, so this would only be used for the fast/success path? But I don't feel strongly.

Sure, but I still think its good to be explicit here. I think passing zero is the cleanest way. But you could also call it quick_exit_with_success but that seems like more typing.

Added explicit 0.

sbc100 · 2026-01-16T00:39:27Z

src/tools/wasm-opt.cpp

      }
    }
+
+    flush_and_quick_exit();


I guess you should flag flush_and_quick_exit as [[noreturn]] ?

sbc100

lgtm with return code passed for explicitness :)

dschuff · 2026-01-16T21:04:03Z

One other thing that just occurred to me: do we run this code under ASan/LSan? is this going to cause false leak reports there?

dschuff

LGTM.
I don't love that the destructors in the main functions of the tools now essentially become manual, that seems error-prone. But hopefully those mains can stay simple enough that it won't matter. I guess if we decide we don't like that, we could try to go for an "opt-out" rather than "opt-in" global destruction approach, where we write some kind of destructor that intentionally leaks those problematically-slow allocations, and then we are safer by default.

sbc100 · 2026-01-16T21:12:04Z

LGTM. I don't love that the destructors in the main functions of the tools now essentially become manual, that seems error-prone. But hopefully those mains can stay simple enough that it won't matter. I guess if we decide we don't like that, we could try to go for an "opt-out" rather than "opt-in" global destruction approach, where we write some kind of destructor that intentionally leaks those problematically-slow allocations, and then we are safer by default.

More likely it might defeat lsan completely by exiting before lsan can produce its report?

I think the hard exit like this is probably a good approach since its what we do in lld: https://github.com/llvm/llvm-project/blob/e36ddff7a475bd18d68fbdaa6e4a8ad924f85490/lld/Common/ErrorHandler.cpp#L105-L108

In llvm we use the lower level C function _Exit, but I think it amounts the same thing: https://github.com/llvm/llvm-project/blob/e36ddff7a475bd18d68fbdaa6e4a8ad924f85490/llvm/lib/Support/Unix/Process.inc#L433C27-L433C28

kripken · 2026-01-16T21:19:02Z

I don't love that the destructors in the main functions of the tools now essentially become manual, that seems error-prone.

Yeah, I don't love this either... If it wasn't actually 10% faster I would not consider it.

As you said, these main functions are at least pretty simple, and we don't have a lot of them.

kripken · 2026-01-16T21:23:50Z

Good points about lsan. I added code to avoid quick_exit when sanitizers run (shutdown speed doesn't matter there anyhow). I'll also verify manually that leak checks still work.

This reverts commit 387ffb9.

kripken · 2026-01-17T00:24:44Z

LSan confirmed still functional.

kripken · 2026-01-17T00:32:58Z

Re-measured locally to confirm the 10% speedup remains after all the changes here.

kripken added 4 commits January 15, 2026 15:44

start

a4ccef6

nice

1728cd7

opos

a6dc74d

fmt

c6732cb

kripken requested review from dschuff and sbc100 January 16, 2026 00:18

sbc100 reviewed Jan 16, 2026

View reviewed changes

dschuff reviewed Jan 16, 2026

View reviewed changes

sbc100 reviewed Jan 16, 2026

View reviewed changes

fix

2a9e76b

sbc100 reviewed Jan 16, 2026

View reviewed changes

[[noreturn]]

4b73e35

sbc100 reviewed Jan 16, 2026

View reviewed changes

kripken added 2 commits January 16, 2026 11:04

explicit 0

8afe5b1

add C flushing

2db41f6

dschuff approved these changes Jan 16, 2026

View reviewed changes

sanitizers

9a4a61f

kripken added 2 commits January 16, 2026 15:57

cause lsan errors?

387ffb9

Revert "cause lsan errors?"

2cb6910

This reverts commit 387ffb9.

[NFC] Use quick_exit to skip global cleanup #8212

Are you sure you want to change the base?

[NFC] Use quick_exit to skip global cleanup #8212

Conversation

kripken commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschuff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

dschuff commented Jan 16, 2026

Uh oh!

dschuff left a comment

Choose a reason for hiding this comment

Uh oh!

sbc100 commented Jan 16, 2026

Uh oh!

kripken commented Jan 16, 2026

Uh oh!

kripken commented Jan 16, 2026

Uh oh!

kripken commented Jan 17, 2026

Uh oh!

kripken commented Jan 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kripken Jan 16, 2026 •

edited

Loading