Skip to content

Commit 2c91a6d

Browse files
sylvestresylvestre
committed
server: remove redundant async block in start_compile_task
Drop the extra `async move { ... }.await` wrapper (and its #[allow(clippy::redundant_async_block)] and TODO) that was kept only to minimize the whitespace diff against upstream/main, and de-indent the task body one level. Purely a cleanup; no behavior change.
1 parent 4411cd0 commit 2c91a6d

1 file changed

Lines changed: 177 additions & 184 deletions

File tree

‎src/server.rs‎

Lines changed: 177 additions & 184 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,218 +1460,211 @@ where
14601460

14611461
let me = self.clone();
14621462

1463-
// This redundant async block exists to reduce whitespace-only
1464-
// changes when comparing this diff with upstream/main.
1465-
// TODO: remove this before merging
1466-
#[allow(clippy::redundant_async_block)]
14671463
util::spawn_on(&self.rt, async move {
1468-
async move {
1469-
let result = match me.dist_client.get_client().await {
1470-
Ok(client) => std::panic::AssertUnwindSafe(hasher.get_cached_or_compile(
1471-
&me,
1472-
client,
1473-
me.creator.clone(),
1474-
me.storage.clone(),
1475-
arguments,
1476-
cwd,
1477-
env_vars,
1478-
cache_control,
1479-
me.rt.clone(),
1480-
))
1481-
.catch_unwind()
1482-
.await
1483-
.map_err(|e| {
1484-
let panic = e
1485-
.downcast_ref::<&str>()
1486-
.map(|s| &**s)
1487-
.or_else(|| e.downcast_ref::<String>().map(|s| &**s))
1488-
.unwrap_or("An unknown panic was caught.");
1489-
let thread = std::thread::current();
1490-
let thread_name = thread.name().unwrap_or("unnamed");
1491-
if let Some((file, line, column)) = PANIC_LOCATION.with(|l| l.take()) {
1492-
anyhow!(
1493-
"thread '{thread_name}' panicked at {file}:{line}:{column}: {panic}"
1494-
)
1495-
} else {
1496-
anyhow!("thread '{thread_name}' panicked: {panic}")
1497-
}
1498-
})
1499-
.and_then(std::convert::identity),
1500-
Err(e) => Err(e),
1501-
};
1464+
let result = match me.dist_client.get_client().await {
1465+
Ok(client) => std::panic::AssertUnwindSafe(hasher.get_cached_or_compile(
1466+
&me,
1467+
client,
1468+
me.creator.clone(),
1469+
me.storage.clone(),
1470+
arguments,
1471+
cwd,
1472+
env_vars,
1473+
cache_control,
1474+
me.rt.clone(),
1475+
))
1476+
.catch_unwind()
1477+
.await
1478+
.map_err(|e| {
1479+
let panic = e
1480+
.downcast_ref::<&str>()
1481+
.map(|s| &**s)
1482+
.or_else(|| e.downcast_ref::<String>().map(|s| &**s))
1483+
.unwrap_or("An unknown panic was caught.");
1484+
let thread = std::thread::current();
1485+
let thread_name = thread.name().unwrap_or("unnamed");
1486+
if let Some((file, line, column)) = PANIC_LOCATION.with(|l| l.take()) {
1487+
anyhow!(
1488+
"thread '{thread_name}' panicked at {file}:{line}:{column}: {panic}"
1489+
)
1490+
} else {
1491+
anyhow!("thread '{thread_name}' panicked: {panic}")
1492+
}
1493+
})
1494+
.and_then(std::convert::identity),
1495+
Err(e) => Err(e),
1496+
};
15021497

1503-
let mut cache_write = None;
1504-
let mut res = CompileFinished {
1505-
color_mode,
1506-
..Default::default()
1507-
};
1498+
let mut cache_write = None;
1499+
let mut res = CompileFinished {
1500+
color_mode,
1501+
..Default::default()
1502+
};
15081503

1509-
let mut stats = me.stats.lock().await;
1504+
let mut stats = me.stats.lock().await;
15101505

1511-
match result {
1512-
Ok((compiled, out)) => {
1513-
let mut dist_type = DistType::NoDist;
1506+
match result {
1507+
Ok((compiled, out)) => {
1508+
let mut dist_type = DistType::NoDist;
15141509

1515-
match compiled {
1516-
CompileResult::Error => {
1517-
debug!("[{}]: compile result: cache error", out_pretty);
1510+
match compiled {
1511+
CompileResult::Error => {
1512+
debug!("[{}]: compile result: cache error", out_pretty);
15181513

1519-
stats.cache_errors.increment(&kind, &lang);
1520-
}
1521-
CompileResult::CacheHit(duration) => {
1522-
debug!("[{}]: compile result: cache hit", out_pretty);
1514+
stats.cache_errors.increment(&kind, &lang);
1515+
}
1516+
CompileResult::CacheHit(duration) => {
1517+
debug!("[{}]: compile result: cache hit", out_pretty);
15231518

1524-
stats.cache_hits.increment(&kind, &lang);
1525-
stats.cache_read_hit_duration += duration;
1526-
}
1527-
CompileResult::CacheMiss(miss_type, dt, duration, future) => {
1528-
debug!("[{}]: compile result: cache miss", out_pretty);
1529-
dist_type = dt;
1530-
1531-
match miss_type {
1532-
MissType::Normal => {}
1533-
MissType::ForcedNoCache => {}
1534-
MissType::ForcedRecache => {
1535-
stats.forced_recaches += 1;
1536-
}
1537-
MissType::TimedOut => {
1538-
stats.cache_timeouts += 1;
1539-
}
1540-
MissType::CacheReadError => {
1541-
stats.cache_errors.increment(&kind, &lang);
1542-
}
1519+
stats.cache_hits.increment(&kind, &lang);
1520+
stats.cache_read_hit_duration += duration;
1521+
}
1522+
CompileResult::CacheMiss(miss_type, dt, duration, future) => {
1523+
debug!("[{}]: compile result: cache miss", out_pretty);
1524+
dist_type = dt;
1525+
1526+
match miss_type {
1527+
MissType::Normal => {}
1528+
MissType::ForcedNoCache => {}
1529+
MissType::ForcedRecache => {
1530+
stats.forced_recaches += 1;
1531+
}
1532+
MissType::TimedOut => {
1533+
stats.cache_timeouts += 1;
1534+
}
1535+
MissType::CacheReadError => {
1536+
stats.cache_errors.increment(&kind, &lang);
15431537
}
1544-
stats.compilations += 1;
1545-
stats.cache_misses.increment(&kind, &lang);
1546-
stats.compiler_write_duration += duration;
1547-
debug!("stats after compile result: {stats:?}");
1548-
cache_write = Some(future);
1549-
}
1550-
CompileResult::NotCached(dt, duration) => {
1551-
debug!("[{}]: compile result: not cached", out_pretty);
1552-
dist_type = dt;
1553-
stats.compilations += 1;
1554-
stats.compiler_write_duration += duration;
1555-
}
1556-
CompileResult::NotCacheable(dt, duration) => {
1557-
debug!("[{}]: compile result: not cacheable", out_pretty);
1558-
dist_type = dt;
1559-
stats.compilations += 1;
1560-
stats.compiler_write_duration += duration;
1561-
stats.non_cacheable_compilations += 1;
1562-
}
1563-
CompileResult::CompileFailed(dt, duration) => {
1564-
debug!("[{}]: compile result: compile failed", out_pretty);
1565-
dist_type = dt;
1566-
stats.compilations += 1;
1567-
stats.compiler_write_duration += duration;
1568-
stats.compile_fails += 1;
15691538
}
1539+
stats.compilations += 1;
1540+
stats.cache_misses.increment(&kind, &lang);
1541+
stats.compiler_write_duration += duration;
1542+
debug!("stats after compile result: {stats:?}");
1543+
cache_write = Some(future);
15701544
}
1571-
1572-
match dist_type {
1573-
DistType::NoDist => {}
1574-
DistType::Ok(id) => {
1575-
let server = id.addr().to_string();
1576-
let server_count = stats.dist_compiles.entry(server).or_insert(0);
1577-
*server_count += 1;
1578-
}
1579-
DistType::Error => stats.dist_errors += 1,
1545+
CompileResult::NotCached(dt, duration) => {
1546+
debug!("[{}]: compile result: not cached", out_pretty);
1547+
dist_type = dt;
1548+
stats.compilations += 1;
1549+
stats.compiler_write_duration += duration;
1550+
}
1551+
CompileResult::NotCacheable(dt, duration) => {
1552+
debug!("[{}]: compile result: not cacheable", out_pretty);
1553+
dist_type = dt;
1554+
stats.compilations += 1;
1555+
stats.compiler_write_duration += duration;
1556+
stats.non_cacheable_compilations += 1;
15801557
}
1558+
CompileResult::CompileFailed(dt, duration) => {
1559+
debug!("[{}]: compile result: compile failed", out_pretty);
1560+
dist_type = dt;
1561+
stats.compilations += 1;
1562+
stats.compiler_write_duration += duration;
1563+
stats.compile_fails += 1;
1564+
}
1565+
}
15811566

1582-
// Make sure the write guard has been dropped ASAP.
1583-
drop(stats);
1567+
match dist_type {
1568+
DistType::NoDist => {}
1569+
DistType::Ok(id) => {
1570+
let server = id.addr().to_string();
1571+
let server_count = stats.dist_compiles.entry(server).or_insert(0);
1572+
*server_count += 1;
1573+
}
1574+
DistType::Error => stats.dist_errors += 1,
1575+
}
15841576

1585-
let Output {
1586-
status,
1587-
stdout,
1588-
stderr,
1589-
} = out;
1577+
// Make sure the write guard has been dropped ASAP.
1578+
drop(stats);
15901579

1591-
trace!("CompileFinished retcode: {}", status);
1580+
let Output {
1581+
status,
1582+
stdout,
1583+
stderr,
1584+
} = out;
15921585

1593-
match status.code() {
1594-
Some(code) => res.retcode = Some(code),
1595-
None => res.signal = Some(get_signal(status)),
1596-
}
1586+
trace!("CompileFinished retcode: {}", status);
15971587

1598-
res.stdout = stdout;
1599-
res.stderr = stderr;
1588+
match status.code() {
1589+
Some(code) => res.retcode = Some(code),
1590+
None => res.signal = Some(get_signal(status)),
16001591
}
1601-
Err(err) => {
1602-
match err.downcast::<ProcessError>() {
1603-
Ok(ProcessError(output)) => {
1604-
debug!("Compilation failed: {:?}", output);
1605-
stats.compile_fails += 1;
1592+
1593+
res.stdout = stdout;
1594+
res.stderr = stderr;
1595+
}
1596+
Err(err) => {
1597+
match err.downcast::<ProcessError>() {
1598+
Ok(ProcessError(output)) => {
1599+
debug!("Compilation failed: {:?}", output);
1600+
stats.compile_fails += 1;
1601+
// Make sure the write guard has been dropped ASAP.
1602+
drop(stats);
1603+
1604+
match output.status.code() {
1605+
Some(code) => res.retcode = Some(code),
1606+
None => res.signal = Some(get_signal(output.status)),
1607+
}
1608+
res.stdout = output.stdout;
1609+
res.stderr = output.stderr;
1610+
}
1611+
Err(err) => match err.downcast::<HttpClientError>() {
1612+
Ok(HttpClientError(msg)) => {
16061613
// Make sure the write guard has been dropped ASAP.
16071614
drop(stats);
1615+
me.dist_client.reset_state().await;
1616+
let errmsg =
1617+
format!("[{:?}] http error status: {}", out_pretty, msg);
1618+
error!("{}", errmsg);
1619+
res.retcode = Some(1);
1620+
res.stderr = errmsg.as_bytes().to_vec();
1621+
}
1622+
Err(err) => {
1623+
stats.cache_errors.increment(&kind, &lang);
1624+
// Make sure the write guard has been dropped ASAP.
1625+
drop(stats);
1626+
1627+
use std::fmt::Write;
16081628

1609-
match output.status.code() {
1610-
Some(code) => res.retcode = Some(code),
1611-
None => res.signal = Some(get_signal(output.status)),
1629+
error!("[{:?}] fatal error: {}", out_pretty, err);
1630+
1631+
let mut error =
1632+
"sccache: encountered fatal error\n".to_string();
1633+
let _ = writeln!(error, "sccache: error: {}", err);
1634+
for e in err.chain() {
1635+
error!("[{:?}] \t{}", out_pretty, e);
1636+
let _ = writeln!(error, "sccache: caused by: {}", e);
16121637
}
1613-
res.stdout = output.stdout;
1614-
res.stderr = output.stderr;
1638+
//TODO: figure out a better way to communicate this?
1639+
res.retcode = Some(-2);
1640+
res.stderr = error.into_bytes();
16151641
}
1616-
Err(err) => match err.downcast::<HttpClientError>() {
1617-
Ok(HttpClientError(msg)) => {
1618-
// Make sure the write guard has been dropped ASAP.
1619-
drop(stats);
1620-
me.dist_client.reset_state().await;
1621-
let errmsg =
1622-
format!("[{:?}] http error status: {}", out_pretty, msg);
1623-
error!("{}", errmsg);
1624-
res.retcode = Some(1);
1625-
res.stderr = errmsg.as_bytes().to_vec();
1626-
}
1627-
Err(err) => {
1628-
stats.cache_errors.increment(&kind, &lang);
1629-
// Make sure the write guard has been dropped ASAP.
1630-
drop(stats);
1631-
1632-
use std::fmt::Write;
1633-
1634-
error!("[{:?}] fatal error: {}", out_pretty, err);
1635-
1636-
let mut error =
1637-
"sccache: encountered fatal error\n".to_string();
1638-
let _ = writeln!(error, "sccache: error: {}", err);
1639-
for e in err.chain() {
1640-
error!("[{:?}] \t{}", out_pretty, e);
1641-
let _ = writeln!(error, "sccache: caused by: {}", e);
1642-
}
1643-
//TODO: figure out a better way to communicate this?
1644-
res.retcode = Some(-2);
1645-
res.stderr = error.into_bytes();
1646-
}
1647-
},
1648-
}
1642+
},
16491643
}
16501644
}
1645+
}
16511646

1652-
if let Some(cache_write) = cache_write {
1653-
match cache_write.await {
1654-
Err(e) => {
1655-
debug!("Error executing cache write: {}", e);
1656-
me.stats.lock().await.cache_write_errors += 1;
1657-
}
1658-
//TODO: save cache stats!
1659-
Ok(info) => {
1660-
debug!(
1661-
"[{}]: Cache write finished in {}",
1662-
info.object_file_pretty,
1663-
util::fmt_duration_as_secs(&info.duration)
1664-
);
1665-
let mut stats = me.stats.lock().await;
1666-
stats.cache_writes += 1;
1667-
stats.cache_write_duration += info.duration;
1668-
}
1647+
if let Some(cache_write) = cache_write {
1648+
match cache_write.await {
1649+
Err(e) => {
1650+
debug!("Error executing cache write: {}", e);
1651+
me.stats.lock().await.cache_write_errors += 1;
1652+
}
1653+
//TODO: save cache stats!
1654+
Ok(info) => {
1655+
debug!(
1656+
"[{}]: Cache write finished in {}",
1657+
info.object_file_pretty,
1658+
util::fmt_duration_as_secs(&info.duration)
1659+
);
1660+
let mut stats = me.stats.lock().await;
1661+
stats.cache_writes += 1;
1662+
stats.cache_write_duration += info.duration;
16691663
}
16701664
}
1671-
1672-
Ok(res)
16731665
}
1674-
.await
1666+
1667+
Ok(res)
16751668
})
16761669
.map_err(anyhow::Error::new)
16771670
.await?

0 commit comments

Comments
 (0)