HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.03k stars 648 forks source link

[hxb] Add define to opt out of hxb cache #11668

Closed kLabz closed 1 month ago

kLabz commented 1 month ago

While I do believe (and can see the results) hxb cache is the way forward for a stable compilation server, in projects where in-memory cache was working fine hxb cache makes display requests slower when in perfect conditions for in-memory cache (no invalidation, cache not getting corrupted).

This PR adds support for -D disable-hxb-cache which caches modules in memory rather than through hxb.

Please note that using this define means you're also opting out of several compilation server instability fixes, and future vshaxe optimizations (requests queue optimization, requests cancellation and potentially concurrent requests). But please do report issues that you cannot reproduce with Haxe 4.3.x.

Simn commented 1 month ago

I guess that define should be on the list of defines that don't change signature, although thinking about that too much makes my head hurt a bit.

kLabz commented 1 month ago

I'm not sure about that :confused:

It seems to me that it's less risky not to have it there, especially if we later need to adjust things depending on whether or not hxb cache is enabled. It's really not a define one should add/remove during the lifetime of the server anyway, imo.

Simn commented 1 month ago

I don't have a particularly strong opinion on this either, but it seems prudent to be able to say "this is the same context" when switching cache mode. If nothing else that might help with debugging down the line.

kLabz commented 1 month ago

Hmm well I do have a bad feeling about that, but nothing concrete yet. We can remove it later if it does end up causing weird issues, I guess.

Simn commented 1 month ago

It might cause issues, but then we'll need to address them anyway if we want to look into having a hybrid cache. That's actually a decent argument for sharing the signature because in theory we could already allow to change this per-module.