http://codereview.chromium.org/10700115/diff/1/src/hydrogen.h File src/hydrogen.h (right):
http://codereview.chromium.org/10700115/diff/1/src/hydrogen.h#newcode248 src/hydrogen.h:248: HGraph(CompilationInfo* info, HGraphBuilder* graph_builder); Don't pass graph_builder here, see longer comment below. http://codereview.chromium.org/10700115/diff/1/src/hydrogen.h#newcode317 src/hydrogen.h:317: LChunk* Optimize(); You have introduced a "layer violation" with this change. HGraph should not have a dependency on anything except for hydrogen instructions. However, with this change you introduce a dependency on the graph builder, which isn't a good idea because the graph shouldn't have knowledge of how it's built, just what it has inside of itself (for example, I am implementing stub generation that uses HGraph, but it doesn't use the HGraphBuilder, which is only used to generate optimized JavaScript functions). You also strengthen an existing dependency on chunks/chunk builder, depending on the Lithium layer from Hydrogen. I'd like to untangle that dependency a little bit if you are moving code already. Concrete suggestion: - Don't keep a pointer to the graph builder in the graph. - Separate the async work into two methods. One on the graph called optimize, which somehow communicates to the caller if it fails due to a bailout, and the caller handles the bailout because it has a reference to the graph builder. Another one called PrepareCodegen, perhaps on the Compiler object, that does the work that you currently have at the end of optimize. - Make the asynchronous part of compilation call "graph()->Optimized() and PrepareCodegen()" http://codereview.chromium.org/10700115/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
