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

Java: the misses in rocksdbjni are very expensive #13023

Open
trazfr opened this issue Sep 19, 2024 · 3 comments
Open

Java: the misses in rocksdbjni are very expensive #13023

trazfr opened this issue Sep 19, 2024 · 3 comments

Comments

@trazfr
Copy link

trazfr commented Sep 19, 2024

Expected behavior

This is only for rocksdbjni.

In rocksdbjni, I would expect the misses to be less expensive than the hits, or at least as expensive.
I have tested v9.5.2, but the last v9.6.1 has no change in java/rocksjni/rocksjni.cc so the issue remains.

Actual behavior

The misses are far more expensive than the hits due to a C++ exception which is thrown then caught a few lines after.

For instance in java/rocksjni/rocksjni.cc in Java_org_rocksdb_RocksDB_get__J_3BII():

    // throws a rocksdb::KVException...
    ROCKSDB_NAMESPACE::KVException::ThrowOnError(
        env,
        db->Get(ROCKSDB_NAMESPACE::ReadOptions(), db->DefaultColumnFamily(),
                key.slice(), &value.pinnable_slice()));
    return value.NewByteArray();

  } catch (ROCKSDB_NAMESPACE::KVException&) {
    // ... which is caught a few lines after
    return nullptr;
  }

Just changing the above code to the following speeds up a lot the misses:

    const auto status =
        db->Get(ROCKSDB_NAMESPACE::ReadOptions(), db->DefaultColumnFamily(),
                key.slice(), &value.pinnable_slice()));
    if (status.ok()) {
      return value.NewByteArray();
    } else if (!status.IsNotFound()) {
      ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(env, status);
    }
  } catch (ROCKSDB_NAMESPACE::KVException&) {
  }
  return nullptr;

Steps to reproduce the behavior

I have created a small reproducer in trazfr/rocksdbjni-miss-perfs which runs the two following rocksdbjni jar and send the results to the Datadog profiler:

  • the vanilla rocksdb 9.5.2
  • a patched version with only the change above
# needs docker-compose
./run.sh --api-key <DATADOG_API_KEY> --site <DATADOG_SITE>

In this reproducer, we create a DB with 1M entries, then we perform:

  • 1M hits with all the keys in the DB (method App.readDatabaseHits())
  • 1M misses (method App.readDatabaseMisses())
  • loop

I don't compare vanilla vs patched as the compilation parameters are different, but only the CPU time spent on hits vs misses in each case. The following tests are done on an AWS m5d.4xlarge machine (linux/x84_64), but I have also been able to reproduce an a M1 Mac (MacOS/aarch64 and qemu+linux/aarch64).

On the vanilla version, in case of miss, we can see that a lot of CPU time is spent in rocksdb::KVException::ThrowOnError(). This makes the misses slower than the hits by far:

image

With the small patch above, as we don't throw exceptions, the misses are a bit less expensive than the hits:

image

@rhubner
Copy link
Contributor

rhubner commented Sep 20, 2024

Hello @trazfr,

thank you for amazing report. I agree that using exceptions for misses is probably not the best. I will check with my colleagues and see how we can optimize this. Probably leave exception on for error cases.

Radek

cc: @adamretter

@adamretter
Copy link
Collaborator

adamretter commented Sep 21, 2024

Thanks @rhubner, it looks like this try/catch approach in the C++ code was originally pioneered by @alanpaxton (see 5a063ec). @alanpaxton Would you be able to take a look at this please? I think a good first step would be to construct some JMH Benchmarks outside of RocksDB in our JNI Benchmarks project to look at the different approaches we could use here and their performance, before we decide how to move forward here. What do you think?

@adamretter
Copy link
Collaborator

@trazfr from your report and attached images I can't seem to ascertain the numbers for the performance difference. Would you be able to tell me a bit more in detail about the performance difference please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants