Skip to content

Enable strict compiler settings #413

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

Merged
merged 42 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
15e39ec
enable compiler warnings and linting
bpkroth Dec 7, 2023
d66eaba
address some trailing whitespace warnings
bpkroth Dec 7, 2023
9fde340
Merge branch 'main' into strict-compiler
bpkroth Dec 8, 2023
3b3906c
compiler fixups
bpkroth Dec 8, 2023
092e06d
compiler fixups
bpkroth Dec 8, 2023
8038636
add some warning suppressions for unused variables
bpkroth Dec 8, 2023
139c953
omit those entirely
bpkroth Dec 8, 2023
259cca6
fixups
bpkroth Dec 8, 2023
77c74ae
whitespace
bpkroth Dec 8, 2023
7abe2e8
use autoclose
bpkroth Dec 8, 2023
a3f7fec
remove more suppressed cast issues
bpkroth Dec 8, 2023
554406d
unnecessary cast
bpkroth Dec 8, 2023
79e7187
avoid unused errors
bpkroth Dec 8, 2023
d93caf7
add serialization requirements
bpkroth Dec 8, 2023
7f89237
whitespace
bpkroth Dec 8, 2023
ff5507d
more minor avoid raw type issues
bpkroth Dec 8, 2023
fc97c1e
in the couple of utility classes, just suppress more warnings
bpkroth Dec 8, 2023
5220c3f
convert throw type to avoid InterruptedException handling warnings
bpkroth Dec 8, 2023
df432f3
docs
bpkroth Dec 8, 2023
d93c61d
enable deprecations
bpkroth Dec 8, 2023
b0398b1
more compiler warning fixups
bpkroth Dec 8, 2023
db891e9
Merge branch 'main' into strict-compiler
bpkroth Dec 8, 2023
c53ea80
revert order
bpkroth Dec 8, 2023
e5e9c00
need the col++
bpkroth Dec 8, 2023
94385e7
address some unused and other lint issues
bpkroth Dec 8, 2023
448ca24
fixup
bpkroth Dec 8, 2023
1e411d4
fixup
bpkroth Dec 8, 2023
2937b46
add a readme with additional developer notes per PR suggestion
bpkroth Dec 11, 2023
4b22f8d
tweak comment
bpkroth Dec 11, 2023
fa34754
add more developer notes
bpkroth Dec 11, 2023
be0f414
include some extension recommendations
bpkroth Dec 11, 2023
43b5cee
update extensions
bpkroth Dec 11, 2023
4556f10
address more unused warnings
bpkroth Dec 11, 2023
5939e2d
ignore another error
bpkroth Dec 11, 2023
4f0fe4c
Merge branch 'main' into strict-compiler
bpkroth Dec 13, 2023
ebb52de
addressing some new serialization warnings
bpkroth Dec 13, 2023
adb1a96
comments
bpkroth Dec 13, 2023
acd47aa
adjust constructor to avoid this-escape errors
bpkroth Dec 13, 2023
d350c6e
convert a pile of classes to 'final' to avoid this-escape errors
bpkroth Dec 13, 2023
3437b2d
auto-whitespace
bpkroth Dec 13, 2023
b2e7489
comments about this-escape warnings
bpkroth Dec 13, 2023
b526294
exception
bpkroth Dec 13, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@
"settings": {},
// Add the IDs of extensions you want installed when the container is created.
"extensions": [
"github.vscode-github-actions",
"EditorConfig.EditorConfig",
"GitHub.copilot",
"eamodio.gitlens",
"vscjava.vscode-java-pack",
"EditorConfig.EditorConfig"
"github.vscode-github-actions",
"github.vscode-pull-request-github",
"huntertran.auto-markdown-toc",
"vscjava.vscode-java-pack"
]
}
},
Expand Down
8 changes: 8 additions & 0 deletions .vscode/extensions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"recommendations": [
"EditorConfig.EditorConfig",
"github.vscode-pull-request-github",
"huntertran.auto-markdown-toc",
"vscjava.vscode-java-pack"
]
}
72 changes: 72 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Contributing

We welcome all contributions! Please open a [pull request](https://github.com/cmu-db/benchbase/pulls). Common contributions may include:

- Adding support for a new DBMS.
- Adding more tests of existing benchmarks.
- Fixing any bugs or known issues.

## Contents

<!-- TOC -->

- [Contributing](#contributing)
- [Contents](#contents)
- [IDE](#ide)
- [Adding a new DBMS](#adding-a-new-dbms)
- [Java Development Notes](#java-development-notes)
- [Avoid var keyword](#avoid-var-keyword)
- [Compiler Warnings](#compiler-warnings)
- [Alternatives to arrays of generics](#alternatives-to-arrays-of-generics)

<!-- /TOC -->

## IDE

Although you can use any IDE you prefer, there are some configurations for [VSCode](https://code.visualstudio.com/) that you may find useful included in the repository, including [Github Codespaces](https://github.com/features/codespaces) and [VSCode devcontainer](https://code.visualstudio.com/docs/remote/containers) support to automatically handle dependencies, environment setup, code formatting, and more.

## Adding a new DBMS

Please see the existing MySQL and PostgreSQL code for an example.

## Java Development Notes

### Compiler Warnings

In an effort to enforce clean, safe, maintainable code, [PR #413](https://github.com/cmu-db/benchbase/pull/413) enabled the `-Werror` and `-Xlint:all` options for the `javac` compiler.

This means that any compiler warnings will cause the build to fail.

If you are seeing a build failure due to a compiler warning, please fix the warning or (on rare occassions) add an exception to the line causing the issue.

### Avoid `var` keyword

In general, we prefer to avoid the `var` keyword in favor of explicit types.

#### Alternatives to arrays of generics

Per the [Java Language Specification](https://docs.oracle.com/javase/tutorial/java/generics/restrictions.html#createArrays), arrays of generic types are not allowed and will cause compiler warnings (e.g., `unchecked cast`)

In some cases, you can extend a generic type to create a non-generic type that can be used in an array.

For instance,

```java
// Simple generic class overload to avoid some cast warnings.
class SomeTypeList extends LinkedList<SomeType> {}

SomeTypeList[] someTypeLists = new SomeTypeList[] {
new SomeTypeList(),
new SomeTypeList(),
new SomeTypeList(),
};
```

#### `this-escape` warnings

`possible 'this' escape before subclass is fully initialized`

The `this-escape` warning above is caused by passing using `this.someOverridableMethod()` in a constructor.
This could in theory cause problems with a subclass not being fully initialized when the method is called.

Since many of our classes are not designed to be subclassed, we can safely ignore this warning by marking the class as `final` rather than completely rewrite the class initialization.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,17 @@ Please see the existing MySQL and PostgreSQL code for an example.

## Contributing

We welcome all contributions! Please open a pull request. Common contributions may include:
We welcome all contributions! Please open a [pull request](https://github.com/cmu-db/benchbase/pulls). Common contributions may include:

- Adding support for a new DBMS.
- Adding more tests of existing benchmarks.
- Fixing any bugs or known issues.

Please see the [CONTRIBUTING.md](./CONTRIBUTING.md) for addition notes.

## Known Issues

Please use GitHub's issue tracker for all issues.
Please use [GitHub's issue tracker](https://github.com/cmu-db/benchbase/issues) for all issues.

## Credits

Expand Down
14 changes: 14 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,20 @@
<configuration>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.target}</target>
<showDeprecation>true</showDeprecation>
<showWarnings>true</showWarnings>
<compilerArgs>
<!-- Enable all linter messages for the compiler. -->
<!-- TODO: Remove annotations preprocess warning exception. -->
<arg>-Xlint:all,-processing</arg>
<!-- ignore annotation processing on generated class files -->
<arg>-implicit:class</arg>
<!-- TODO: Enable documentation linting.
<arg>-Xdoclint:all</arg>
-->
<!-- Turn all warnings into errors to help keep the code clean. -->
<arg>-Werror</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.concurrent.TimeUnit;

public class DistributionStatistics {
@SuppressWarnings("unused")
private static final Logger LOG = LoggerFactory.getLogger(DistributionStatistics.class);

private static final double[] PERCENTILES = {0.0, 0.25, 0.5, 0.75, 0.9, 0.95, 0.99, 1.0};
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/oltpbenchmark/WorkloadState.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class WorkloadState {
private final Iterator<Phase> phaseIterator;

private int workersWaiting = 0;
@SuppressWarnings("unused") // never read
private int workersWorking = 0;
private int workerNeedSleep;

Expand All @@ -60,7 +61,7 @@ public WorkloadState(BenchmarkState benchmarkState, List<Phase> works, int num_t
*/
public void addToQueue(int amount, boolean resetQueues) {
int workAdded = 0;

synchronized (this) {
if (resetQueues) {
workQueue.clear();
Expand All @@ -71,7 +72,7 @@ public void addToQueue(int amount, boolean resetQueues) {
|| !currentPhase.isRateLimited() || currentPhase.isSerial()) {
return;
}

// Add the specified number of procedures to the end of the queue.
// If we can't keep up with current rate, truncate transactions
for (int i = 0; i < amount && workQueue.size() <= RATE_QUEUE_LIMIT; ++i) {
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/oltpbenchmark/api/BenchmarkModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ public abstract class BenchmarkModule {
public BenchmarkModule(WorkloadConfiguration workConf) {
this.workConf = workConf;
this.dialects = new StatementDialects(workConf);
initClassLoader();
// setClassLoader();
this.classLoader = ClassLoader.getSystemClassLoader();
}

/**
* Instantiates the classLoader variable, needs to be overwritten if
* benchmark uses a custom implementation.
*/
protected void initClassLoader() {
protected void setClassLoader() {
this.classLoader = ClassLoader.getSystemClassLoader();
}

Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/oltpbenchmark/api/StatementDialects.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.oltpbenchmark.WorkloadConfiguration;
import com.oltpbenchmark.api.dialects.*;
import com.oltpbenchmark.types.DatabaseType;
import com.oltpbenchmark.types.State;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xml.sax.SAXException;
Expand All @@ -30,7 +29,6 @@
import javax.xml.transform.stream.StreamSource;
import javax.xml.validation.Schema;
import javax.xml.validation.SchemaFactory;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringWriter;
Expand All @@ -40,7 +38,7 @@
/**
* @author pavlo
*/
public class StatementDialects {
public final class StatementDialects {
private static final Logger LOG = LoggerFactory.getLogger(StatementDialects.class);

private static final DatabaseType DEFAULT_DB_TYPE = DatabaseType.MYSQL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@
import java.util.ArrayList;
import java.util.List;

public class AuctionMarkBenchmark extends BenchmarkModule {

public final class AuctionMarkBenchmark extends BenchmarkModule {
@SuppressWarnings("unused")
private static final Logger LOG = LoggerFactory.getLogger(AuctionMarkBenchmark.class);


private final RandomGenerator rng = new RandomGenerator((int) System.currentTimeMillis());

public AuctionMarkBenchmark(WorkloadConfiguration workConf) {
Expand Down Expand Up @@ -70,6 +69,4 @@ protected List<Worker<? extends BenchmarkModule>> makeWorkersImpl() {
protected Loader<AuctionMarkBenchmark> makeLoaderImpl() {
return new AuctionMarkLoader(this);
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
* @author pavlo
* @author visawee
*/
public class AuctionMarkLoader extends Loader<AuctionMarkBenchmark> {
public final class AuctionMarkLoader extends Loader<AuctionMarkBenchmark> {

// -----------------------------------------------------------------
// INTERNAL DATA MEMBERS
Expand Down Expand Up @@ -1195,6 +1195,7 @@ protected class ItemBidGenerator extends SubTableGenerator<LoaderItemInfo> {
private LoaderItemInfo.Bid bid = null;
private float currentBidPriceAdvanceStep;
private long currentCreateDateAdvanceStep;
@SuppressWarnings("unused") // only ever assigned
private float currentPrice;
private boolean new_item;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public class AuctionMarkProfile {
// ----------------------------------------------------------------
// SERIALIZABLE DATA MEMBERS
// ----------------------------------------------------------------
static final long serialVersionUID = 0;

/**
* Database Scale Factor
Expand Down Expand Up @@ -108,6 +109,14 @@ public class AuctionMarkProfile {
*/
protected transient Histogram<Integer> items_per_category = new Histogram<>();

/**
* Simple generic class overload to avoid some cast warnings below.
*/
class ItemInfoList extends LinkedList<ItemInfo> {
// Required serialization field.
static final long serialVersionUID = 0;
}

/**
* Three status types for an item:
* (1) Available - The auction of this item is still open
Expand All @@ -117,13 +126,13 @@ public class AuctionMarkProfile {
* (3) Complete (The auction is closed and (There is no bid winner or
* the bid winner has already purchased the item)
*/
private transient final LinkedList<ItemInfo> items_available = new LinkedList<>();
private transient final LinkedList<ItemInfo> items_endingSoon = new LinkedList<>();
private transient final LinkedList<ItemInfo> items_waitingForPurchase = new LinkedList<>();
private transient final LinkedList<ItemInfo> items_completed = new LinkedList<>();
private transient final ItemInfoList items_available = new ItemInfoList();
private transient final ItemInfoList items_endingSoon = new ItemInfoList();
private transient final ItemInfoList items_waitingForPurchase = new ItemInfoList();
private transient final ItemInfoList items_completed = new ItemInfoList();


@SuppressWarnings("unchecked")
protected transient final LinkedList<ItemInfo>[] allItemSets = new LinkedList[]{
protected transient final ItemInfoList allItemSets[] = new ItemInfoList[]{
this.items_available,
this.items_endingSoon,
this.items_waitingForPurchase,
Expand Down Expand Up @@ -286,7 +295,7 @@ private AuctionMarkProfile copyProfile(AuctionMarkWorker worker, AuctionMarkProf
this.items_per_category = other.items_per_category;
this.gag_ids = other.gag_ids;

// Initialize the UserIdGenerator so we can figure out whether our
// Initialize the UserIdGenerator so we can figure out whether our
// client should even have these ids
this.initializeUserIdGenerator(this.client_id);

Expand Down Expand Up @@ -455,7 +464,6 @@ private static void loadItems(AuctionMarkProfile profile, List<Object[]> vt) {

private static void loadPendingItemComments(AuctionMarkProfile profile, List<Object[]> vt) {
for (Object[] row : vt) {
int col = 1;
long ic_id = SQLUtil.getLong(row[0]);
String ic_i_id = SQLUtil.getString(row[1]);
String ic_u_id = SQLUtil.getString(row[2]);
Expand Down Expand Up @@ -712,6 +720,7 @@ private boolean addItem(LinkedList<ItemInfo> items, ItemInfo itemInfo) {
// HACK: Always swap existing ItemInfos with our new one, since it will
// more up-to-date information
ItemInfo existing = items.set(idx, itemInfo);
assert existing != null;

return (true);
}
Expand Down Expand Up @@ -861,7 +870,7 @@ private ItemInfo getRandomItem(LinkedList<ItemInfo> itemSet, boolean needCurrent
continue;
}

// If they want an item that is ending in the future, then we compare it with
// If they want an item that is ending in the future, then we compare it with
// the current timestamp
if (needFutureEndDate) {
boolean compareTo = (temp.getEndDate().compareTo(currentTime) < 0);
Expand Down
Loading