search

IITH-Compilers / IR2Vec

Implementation of IR2Vec, LLVM IR Based Scalable Program Embeddings
Other
84 stars 39 forks source link

SQLite benchmark #113

Closed m-atalla closed 4 months ago

m-atalla commented 5 months ago

Hi,

This PR adds SQLite Amalgamation as a benchmark to test IR2Vec performance.

@svkeerthy I have a couple of points that I'm not sure about:

  1. Should this benchmark be conditionally enabled with a flag?
  2. Is it OK to leave the SQLite source files in the build directory?

Let me know if you have any additional suggestions.

Thanks, Mohamed

svkeerthy commented 5 months ago

Hi @m-atalla,

Thanks for the PR!

  1. Yes, it would be good to conditionally enable it. It is fine to have it ON by default.
  2. It is fine to leave the source in the build directory under test directory.

Additionally, I think your PR would be a good to value addition to the functional tests as well. This can be done by linking it with lit and test for the expected embeddings, just like what we currently do for PE-Benchmarks. Please let me know if you have any questions.

Thanks, Venkat

m-atalla commented 5 months ago

Thanks for your feedback @svkeerthy.

I'm not very familiar with how lit works here, it looks like a simple test runner. Anyhow, I think doing tests function and program level tests are rather straight forward. However, onDemand function level seems bit more involved as I'll have to extract all functions of the SQLite.

Unfortunately, I don't have much free time this week, so I have turned this PR into a draft for now, will open it again and ping you once this is ready.

m-atalla commented 5 months ago

@svkeerthy

svkeerthy commented 4 months ago

Hi @m-atalla,

Apologies for the delay. I was caught up with some deadlines.

Thanks for the changes. The code looks good. Weirdly, I could not see "Test Passed" or "Test Failed" logs for SQLite in the tests action eventhough the ENABLE_SQLITE option is set to ON. Am I missing something?

m-atalla commented 4 months ago

Correct, the tests status should be reported in the runner, it looks like the IR file is being generated correctly. Let me double check and verify that its working as intended.

m-atalla commented 4 months ago

@svkeerthy this is fixed, the test action is working correctly now.

Sorry I had some cached build files that made this hard to debug on my end. Anyways, All I needed was to re-order the CMake commands so SQLite stuff is generated before the test script is configured and the benchmarks files are copied.

I think next I can look into profiling so that we know what portions of IR2Vec could benefit the most from parallelizing. I will share my findings in #101 by next Friday if I find anything worth sharing.

svkeerthy commented 4 months ago

Great! Sounds good.