Review flag NTBUILD

[rhuanjl]

The flag NTBUILD is used to guard various features included in chakra.dll but not in chakracore, I believe that most of the code behind it is not functional without additional code that is not in this repository and is unlikely to be made available.

We should therefore review and consider deleting as dead code anything wrapped with #ifdef NTBUILD. There is a possibility that some of this code could be usable for some other purpose - hence a review rather than immediate deletion.

Additionally the following flags are turned on by enabling NTBUILD - each of these should be individually reviewed and any code behind them considered for deletion as dead, again should consider in each case if the code can be repurposed or used without the additional code this repository does not contain:

• ENABLE_PROJECTION
• ENABLE_FOUNDATION_OBJECT
• ENABLE_EXPERIMENTAL_FLAGS
• ENABLE_WININET_PROFILE_DATA_CACHE
• ENABLE_COMPRESSION_UTILITIES
• ENABLE_BASIC_TELEMETRY
• ENABLE_DOM_FAST_PATH
• EDIT_AND_CONTINUE
• ENABLE_JIT_CLAMP
• ENABLE_SCRIPT_PROFILING

[Penguinwizzard]

Some info off the top of my head:
DOM_FAST_PATH was for stuff to speed up interactions with the Spartan DOM - I don't think anyone will miss it. It was basically added initially due to high overhead in calls back to Spartan for DOM queries, iirc.
BASIC_TELEMETRY was for integrating with a telemetry component in ChakraFull to get loose stats on how frequent use of various builtins was; it can be removed.
EDIT_AND_CONTINUE is a neat debugging feature where you can modify code when stopped at a breakpoint; this should be kept for debuggers.
SCRIPT_PROFILING iirc is related to getting script performance information for the Spartan debugger, but I'm not sure.

[rhuanjl]

Thanks for the info @Penguinwizzard

[digitalinfinity]

ENABLE_FOUNDATION_OBJECT is for chakra.dll, not chakracore, and was used to decide whether the WinRT foundation namespace was exposed in script. It can probably be removed.
ENABLE_WININET_PROFILE_DATA_CACHE is also windows only, and was used by Edge when profile data was cached. It might be useful in other scenarios too but at the very least, I'd rename the macro.
ENABLE_COMPRESSION_UTILITIES is used by the parser state cache- it's more recent, I think it's probably useful to ChakraCore, @boingoing should weigh in
ENABLE_JIT_CLAMPING iirc allowed controlling whether threads were allowed to JIT code or not. It probably is still useful too.
ENABLE_SCRIPT_PROFILING was used for the F12 profiler in the browser. It's probably useful to see how chakracore could plug in to profilers but honestly, it's probably not super helpful, iirc it effectively turned off the jit when profiling was enabled, so it's probably worth removing this code.

[ppenzin]

DOM_FAST_PATH was for stuff to speed up interactions with the Spartan DOM - I don't think anyone will miss it. It was basically added initially due to high overhead in calls back to Spartan for DOM queries, iirc.

@Penguinwizzard, would this be useful if somebody would ever integrate ChakraCore into a browser?

[digitalinfinity]

@ppenzin my two cents- while it could be useful, I'd remove it and revisit it later as a native to JS FFI fast-path thats not tied to browsers.

[Penguinwizzard]

I agree; DOM_FAST_PATH is a little too browser-specific to really be something I'd recommend using for new code; if implemented in the current state of the repo, you could probably come up with something more efficient and easier to work with.

[ppenzin]

@digitalinfinity @Penguinwizzard, thank you for the input! This code got removed in #6564.

[boingoing]

ENABLE_WININET_PROFILE_DATA_CACHE is also used to control when the parser state cache is enabled in ChakraFull - for the browser it requires WININET as the state cache is stored in the WININET cache. In ChakraCore we never have this cache available and host needs to pass the state cache buffer manually. Thus can probably remove the WININET related code here as well.

ENABLE_COMPRESSION_UTILITIES is only used for the parser state cache and only supported in Windows in ChakraFull. It wasn't much more than an experiment, ultimately, and the compression feature is not otherwise configurable. This is probably safe to delete entirely.