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

Requesting bytecode/IR for one contract results in it being generated for all selected contracts #15373

Open
klkvr opened this issue Aug 29, 2024 · 4 comments · May be fixed by #15433
Open

Requesting bytecode/IR for one contract results in it being generated for all selected contracts #15373

klkvr opened this issue Aug 29, 2024 · 4 comments · May be fixed by #15433
Labels
medium effort Default level of effort medium impact Default level of impact must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. performance 🐎

Comments

@klkvr
Copy link

klkvr commented Aug 29, 2024

Description

I am attaching 2 standard JSON input files. Both of them include two sources. One of them (src/contract.sol) is a Uniswap V3 position manager (can be any contract that takes at least some time to compile). Second is a contract containing a single import directive, importing the heavy contract (import "src/contract.sol";)

In first input, bytecode output selection is requested for both contracts, and in second input - only for the second one.

input1.json
input2.json

Expected behavior

I would expect solc to skip compiling contracts which are excluded from output selection unless they appear as a bytecode dependency (new keyword or .creationCode) in some of the included contracts.

Actual behavior

It seems that both inputs take a similar pretty long time to compile event though second input basically just compiles an empty source, and outputs nothing. I assume it spends time compiling imported heavy contract

$ time cat input1.json | solc --standard-json
solc --standard-json  3.49s user 0.09s system 96% cpu 3.707 total
$ time cat input2.json | solc --standard-json
solc --standard-json  3.49s user 0.07s system 99% cpu 3.597 total

Fixing this would mean a potentially very high compile time optimization. Right now, even if tooling resolves contract dependecies, and excludes all of them from output selection, solc still spends time compiling and optimizing unused dependencies, even though the resulted bytecode is not outputted at all.

Environment

  • Compiler version: 0.8.26
  • Target EVM version (as per compiler settings): Paris
  • Operating system: macOS
@klkvr klkvr added the bug 🐛 label Aug 29, 2024
@klkvr klkvr changed the title solc seems to spend time compiling excluded contracts solc seems to spend time compiling unused contracts Aug 29, 2024
@klkvr klkvr changed the title solc seems to spend time compiling unused contracts solc spends time compiling unused contracts Aug 29, 2024
@cameel
Copy link
Member

cameel commented Aug 30, 2024

I can confirm this, but it does not really seem to be a bug, it's more of a limitation of the current design. Not all outputs are generated on-demand - some of them, especially the heavier ones like IR or bytecode, require some upfront work and that work is done in bulk for all contracts even if the output is requested only for one of them.

bool isEvmBytecodeRequested(Json const& _outputSelection)
{
if (!_outputSelection.is_object())
return false;
static std::vector<std::string> const outputsThatRequireEvmBinaries = std::vector<std::string>{
"*",
"evm.gasEstimates", "evm.legacyAssembly", "evm.assembly"
} + evmObjectComponents("bytecode") + evmObjectComponents("deployedBytecode");
for (auto const& fileRequests: _outputSelection)
for (auto const& requests: fileRequests)
for (auto const& output: outputsThatRequireEvmBinaries)
if (isArtifactRequested(requests, output, false))
return true;
return false;
}

So this has nothing to do with the import in your example, but rather with your output selection:

input1.json:

"outputSelection": {
    "src/importer.sol": {
        "*": [
            "abi",
            "evm.bytecode",
            "evm.deployedBytecode",
            "evm.methodIdentifiers",
            "metadata"
        ]
    },
    "src/contract.sol": {
        "*": [
            "abi",
            "evm.bytecode",
            "evm.deployedBytecode",
            "evm.methodIdentifiers",
            "metadata"
        ]
    }
},

input2.json:

"outputSelection": {
    "src/importer.sol": {
        "*": [
            "abi",
            "evm.bytecode",
            "evm.deployedBytecode",
            "evm.methodIdentifiers",
            "metadata"
        ]
    },
    "src/contract.sol": {
        "*": []
    }
},

Both of these currently will perform the same work, the second one will just not output some of the results of that work. That's because src/importer.sol requires bytecode generation and therefore bytecode will be generated for all contracts.

I agree that this is not a great design and we should change it. It's technically possible to generate bytecode only for the contracts you request, it will just require changing the way the artifact selection is done in general, which might take some tedious refactoring.

@cameel cameel added medium effort Default level of effort medium impact Default level of impact must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. performance 🐎 low impact Changes are not very noticeable or potential benefits are limited. and removed bug 🐛 medium impact Default level of impact labels Aug 30, 2024
@cameel
Copy link
Member

cameel commented Aug 30, 2024

Fixing this would mean a potentially very high compile time optimization. Right now, even if tooling resolves contract dependecies, and excludes all of them from output selection, solc still spends time compiling and optimizing unused dependencies, even though the resulted bytecode is not outputted at all.

By the way, do you know of any tool that is actually impacted by it in practice? AFAIK tools typically request the same artifacts for all contracts, which is probably why no one brought this issue up so far. This does have potential to speed things up a lot but only if the compiler is actually being used that way. I'd prioritize this issue higher if you have some evidence that it actually is.

@cameel cameel changed the title solc spends time compiling unused contracts Requesting bytecode/IR for one contract results in it being generated for all selected contracts Aug 30, 2024
@klkvr
Copy link
Author

klkvr commented Aug 30, 2024

By the way, do you know of any tool that is actually impacted by it in practice? AFAIK tools typically request the same artifacts for all contracts, which is probably why no one brought this issue up so far. This does have potential to speed things up a lot but only if the compiler is actually being used that way. I'd prioritize this issue higher if you have some evidence that it actually is.

In Foundry, we erase output selection for contracts which are cached:
https://github.com/foundry-rs/compilers/blob/c16927bf601e464aa8765d31c88bf8ffe5285be6/crates/compilers/src/cache.rs#L661
https://github.com/foundry-rs/compilers/blob/c16927bf601e464aa8765d31c88bf8ffe5285be6/crates/compilers/src/filter.rs#L145

@cameel cameel added medium impact Default level of impact and removed low impact Changes are not very noticeable or potential benefits are limited. labels Aug 30, 2024
@cameel
Copy link
Member

cameel commented Aug 30, 2024

Ok then, in that case it seems worth taking care of sooner rather than later.

@cameel cameel linked a pull request Sep 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium effort Default level of effort medium impact Default level of impact must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. performance 🐎
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants