Skip to content

Commit 960c1ac

Browse files
stepanchegfacebook-github-bot
authored andcommitted
Validate Evaluator::enable_profile call
Summary: Only one of `enable_profile` or `enable_instrumentation` can be called, and at most once. Increase likelihood of failing an integration test during incorrect refactoring. Reviewed By: krallin Differential Revision: D38527560 fbshipit-source-id: 7ee865500247b13481789609c9c2899f3c539b27
1 parent 694f1a7 commit 960c1ac

File tree

3 files changed

+54
-0
lines changed

3 files changed

+54
-0
lines changed

starlark/src/eval/runtime/evaluator.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use std::path::Path;
2525
use anyhow::Context;
2626
use gazebo::any::AnyLifetime;
2727
use gazebo::cast;
28+
use gazebo::dupe::Dupe;
2829
use thiserror::Error;
2930

3031
use crate::codemap::FileSpan;
@@ -49,6 +50,7 @@ use crate::eval::runtime::profile::bc::BcProfile;
4950
use crate::eval::runtime::profile::flame::FlameProfile;
5051
use crate::eval::runtime::profile::heap::HeapProfile;
5152
use crate::eval::runtime::profile::heap::HeapProfileFormat;
53+
use crate::eval::runtime::profile::or_instrumentation::ProfileOrInstrumentationMode;
5254
use crate::eval::runtime::profile::stmt::StmtProfile;
5355
use crate::eval::runtime::profile::typecheck::TypecheckProfile;
5456
use crate::eval::runtime::profile::ProfileMode;
@@ -85,6 +87,8 @@ pub(crate) enum EvaluatorError {
8587
BcProfilingNotEnabled,
8688
#[error("Typecheck profiling not enabled")]
8789
TypecheckProfilingNotEnabled,
90+
#[error("Profile or instrumentation already enabled")]
91+
ProfileOrInstrumentationAlreadyEnabled,
8892
#[error("Top frame is not def (internal error)")]
8993
TopFrameNotDef,
9094
#[error("Top second frame is not def (internal error)")]
@@ -124,6 +128,8 @@ pub struct Evaluator<'v, 'a> {
124128
pub(crate) verbose_gc: bool,
125129
// Size of the heap when we should next perform a GC.
126130
pub(crate) next_gc_level: usize,
131+
// Profiling or instrumentation enabled.
132+
pub(crate) profile_or_instrumentation_mode: ProfileOrInstrumentationMode,
127133
// Extra functions to run on each statement, usually empty
128134
pub(crate) before_stmt: BeforeStmt<'a>,
129135
// Used for line profiling
@@ -174,6 +180,7 @@ impl<'v, 'a> Evaluator<'v, 'a> {
174180
next_gc_level: GC_THRESHOLD,
175181
disable_gc: false,
176182
alloca: Alloca::new(),
183+
profile_or_instrumentation_mode: ProfileOrInstrumentationMode::None,
177184
heap_profile: HeapProfile::new(),
178185
stmt_profile: StmtProfile::new(),
179186
bc_profile: BcProfile::new(),
@@ -212,6 +219,12 @@ impl<'v, 'a> Evaluator<'v, 'a> {
212219
/// Profilers add overhead, and while some profilers can be used together,
213220
/// it's better to run at most one profiler at a time.
214221
pub fn enable_profile(&mut self, mode: &ProfileMode) -> anyhow::Result<()> {
222+
if self.profile_or_instrumentation_mode != ProfileOrInstrumentationMode::None {
223+
return Err(EvaluatorError::ProfileOrInstrumentationAlreadyEnabled.into());
224+
}
225+
226+
self.profile_or_instrumentation_mode = ProfileOrInstrumentationMode::Profile(mode.dupe());
227+
215228
match mode {
216229
ProfileMode::HeapSummary
217230
| ProfileMode::HeapFlame
@@ -255,6 +268,13 @@ impl<'v, 'a> Evaluator<'v, 'a> {
255268
/// This function need to be called when evaluating a dependency of a module, if a module
256269
/// does profiling in the given mode.
257270
pub fn enable_profile_instrumentation(&mut self, mode: &ProfileMode) -> anyhow::Result<()> {
271+
if self.profile_or_instrumentation_mode != ProfileOrInstrumentationMode::None {
272+
return Err(EvaluatorError::ProfileOrInstrumentationAlreadyEnabled.into());
273+
}
274+
275+
self.profile_or_instrumentation_mode =
276+
ProfileOrInstrumentationMode::Instrumentation(mode.dupe());
277+
258278
match mode {
259279
ProfileMode::Bytecode | ProfileMode::BytecodePairs => {
260280
self.bc_profile.enable_1();

starlark/src/eval/runtime/profile/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub(crate) mod csv;
2525
pub(crate) mod flame;
2626
pub(crate) mod flamegraph;
2727
pub(crate) mod heap;
28+
pub(crate) mod or_instrumentation;
2829
pub(crate) mod stmt;
2930
pub(crate) mod typecheck;
3031

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright 2019 The Starlark in Rust Authors.
3+
* Copyright (c) Facebook, Inc. and its affiliates.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
use gazebo::dupe::Dupe;
19+
20+
use crate::eval::ProfileMode;
21+
22+
#[derive(Debug, Clone, Dupe, Eq, PartialEq)]
23+
pub(crate) enum ProfileOrInstrumentationMode {
24+
None,
25+
Instrumentation(ProfileMode),
26+
Profile(ProfileMode),
27+
}
28+
29+
impl Default for ProfileOrInstrumentationMode {
30+
fn default() -> ProfileOrInstrumentationMode {
31+
ProfileOrInstrumentationMode::None
32+
}
33+
}

0 commit comments

Comments
 (0)