Skip to content

Commit

Permalink
Improve interface handling in storage module.
Browse files Browse the repository at this point in the history 
Make the interface object the parent of the driver object rather than the interface being allocated directly in the driver object.

The prior method was more efficient when mem contexts had a much higher cost. Now mem contexts are cheap so it makes more sense to structure the objects in a way that works better with mem context auditing. This also means the mem context does not need to be stored separately since it can be extracted directly from the interface object.

There are other areas that need to get the same improvement before the specialized objMoveContext() and objFreeContext() functions can be removed.
  • Loading branch information
@dwsteele
dwsteele committed Mar 7, 2023
1 parent 120a49b commit f6e3073
Show file tree
Hide file tree
Showing 20 changed files with 242 additions and 241 deletions.
12 changes: 5 additions & 7 deletions src/storage/azure/read.c
View file
Expand Up @@ -123,13 +123,13 @@ storageReadAzureNew(
ASSERT(storage != NULL);
ASSERT(name != NULL);

StorageRead *this = NULL;
StorageReadAzure *this = NULL;

OBJ_NEW_BEGIN(StorageReadAzure, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
OBJ_NEW_BEGIN(StorageReadAzure, .childQty = MEM_CONTEXT_QTY_MAX)
{
StorageReadAzure *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), StorageRead::StorageReadAzure);
this = OBJ_NEW_ALLOC();

*driver = (StorageReadAzure)
*this = (StorageReadAzure)
{
.storage = storage,

Expand All @@ -149,10 +149,8 @@ storageReadAzureNew(
},
},
};

this = storageReadNew(driver, &driver->interface);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE_READ, this);
FUNCTION_LOG_RETURN(STORAGE_READ, storageReadNew(this, &this->interface));
}
32 changes: 15 additions & 17 deletions src/storage/azure/storage.c
View file
Expand Up @@ -734,13 +734,13 @@ storageAzureNew(
ASSERT(key != NULL);
ASSERT(blockSize != 0);

Storage *this = NULL;
StorageAzure *this = NULL;

OBJ_NEW_BEGIN(StorageAzure, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
OBJ_NEW_BEGIN(StorageAzure, .childQty = MEM_CONTEXT_QTY_MAX)
{
StorageAzure *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), Storage::StorageAzure);
this = OBJ_NEW_ALLOC();

*driver = (StorageAzure)
*this = (StorageAzure)
{
.interface = storageInterfaceAzure,
.container = strDup(container),
Expand All @@ -754,32 +754,30 @@ storageAzureNew(

// Store shared key or parse sas query
if (keyType == storageAzureKeyTypeShared)
driver->sharedKey = bufNewDecode(encodingBase64, key);
this->sharedKey = bufNewDecode(encodingBase64, key);
else
driver->sasKey = httpQueryNewStr(key);
this->sasKey = httpQueryNewStr(key);

// Create the http client used to service requests
driver->httpClient = httpClientNew(
this->httpClient = httpClientNew(
tlsClientNewP(
sckClientNew(driver->host, port, timeout, timeout), driver->host, timeout, timeout, verifyPeer, .caFile = caFile,
sckClientNew(this->host, port, timeout, timeout), this->host, timeout, timeout, verifyPeer, .caFile = caFile,
.caPath = caPath),
timeout);

// Create list of redacted headers
driver->headerRedactList = strLstNew();
strLstAdd(driver->headerRedactList, HTTP_HEADER_AUTHORIZATION_STR);
strLstAdd(driver->headerRedactList, HTTP_HEADER_DATE_STR);
this->headerRedactList = strLstNew();
strLstAdd(this->headerRedactList, HTTP_HEADER_AUTHORIZATION_STR);
strLstAdd(this->headerRedactList, HTTP_HEADER_DATE_STR);

// Create list of redacted query keys
driver->queryRedactList = strLstNew();
strLstAdd(driver->queryRedactList, AZURE_QUERY_SIG_STR);
this->queryRedactList = strLstNew();
strLstAdd(this->queryRedactList, AZURE_QUERY_SIG_STR);

// Generate starting file id
cryptoRandomBytes((unsigned char *)&driver->fileId, sizeof(driver->fileId));

this = storageNew(STORAGE_AZURE_TYPE, path, 0, 0, write, pathExpressionFunction, driver, driver->interface);
cryptoRandomBytes((unsigned char *)&this->fileId, sizeof(this->fileId));
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE, this);
FUNCTION_LOG_RETURN(STORAGE, storageNew(STORAGE_AZURE_TYPE, path, 0, 0, write, pathExpressionFunction, this, this->interface));
}
14 changes: 6 additions & 8 deletions src/storage/azure/write.c
View file
Expand Up @@ -262,7 +262,7 @@ storageWriteAzureClose(THIS_VOID)

/**********************************************************************************************************************************/
FN_EXTERN StorageWrite *
storageWriteAzureNew(StorageAzure *storage, const String *name, uint64_t fileId, size_t blockSize)
storageWriteAzureNew(StorageAzure *const storage, const String *const name, const uint64_t fileId, const size_t blockSize)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(STORAGE_AZURE, storage);
Expand All @@ -274,13 +274,13 @@ storageWriteAzureNew(StorageAzure *storage, const String *name, uint64_t fileId,
ASSERT(storage != NULL);
ASSERT(name != NULL);

StorageWrite *this = NULL;
StorageWriteAzure *this = NULL;

OBJ_NEW_BEGIN(StorageWriteAzure, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
OBJ_NEW_BEGIN(StorageWriteAzure, .childQty = MEM_CONTEXT_QTY_MAX)
{
StorageWriteAzure *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), StorageWrite::StorageWriteAzure);
this = OBJ_NEW_ALLOC();

*driver = (StorageWriteAzure)
*this = (StorageWriteAzure)
{
.storage = storage,
.fileId = fileId,
Expand All @@ -304,10 +304,8 @@ storageWriteAzureNew(StorageAzure *storage, const String *name, uint64_t fileId,
},
},
};

this = storageWriteNew(driver, &driver->interface);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE_WRITE, this);
FUNCTION_LOG_RETURN(STORAGE_WRITE, storageWriteNew(this, &this->interface));
}
10 changes: 4 additions & 6 deletions src/storage/gcs/read.c
View file
Expand Up @@ -130,13 +130,13 @@ storageReadGcsNew(
ASSERT(storage != NULL);
ASSERT(name != NULL);

StorageRead *this = NULL;
StorageReadGcs *this = NULL;

OBJ_NEW_BEGIN(StorageReadGcs, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
{
StorageReadGcs *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), StorageRead::StorageReadGcs);
this = OBJ_NEW_ALLOC();

*driver = (StorageReadGcs)
*this = (StorageReadGcs)
{
.storage = storage,

Expand All @@ -156,10 +156,8 @@ storageReadGcsNew(
},
},
};

this = storageReadNew(driver, &driver->interface);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE_READ, this);
FUNCTION_LOG_RETURN(STORAGE_READ, storageReadNew(this, &this->interface));
}
54 changes: 26 additions & 28 deletions src/storage/gcs/storage.c
View file
Expand Up @@ -946,9 +946,9 @@ static const StorageInterface storageInterfaceGcs =

FN_EXTERN Storage *
storageGcsNew(
const String *path, bool write, StoragePathExpressionCallback pathExpressionFunction, const String *bucket,
StorageGcsKeyType keyType, const String *key, size_t chunkSize, const String *endpoint, TimeMSec timeout, bool verifyPeer,
const String *caFile, const String *caPath)
const String *const path, const bool write, StoragePathExpressionCallback pathExpressionFunction, const String *const bucket,
const StorageGcsKeyType keyType, const String *const key, const size_t chunkSize, const String *const endpoint,
const TimeMSec timeout, const bool verifyPeer, const String *const caFile, const String *const caPath)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(STRING, path);
Expand All @@ -970,13 +970,13 @@ storageGcsNew(
ASSERT(keyType == storageGcsKeyTypeAuto || key != NULL);
ASSERT(chunkSize != 0);

Storage *this = NULL;
StorageGcs *this = NULL;

OBJ_NEW_BEGIN(StorageGcs, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
{
StorageGcs *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), Storage::StorageGcs);
this = OBJ_NEW_ALLOC();

*driver = (StorageGcs)
*this = (StorageGcs)
{
.interface = storageInterfaceGcs,
.write = write,
Expand All @@ -991,11 +991,11 @@ storageGcsNew(
// Auto authentication for GCE instances
case storageGcsKeyTypeAuto:
{
driver->authUrl = httpUrlNewParseP(
this->authUrl = httpUrlNewParseP(
STRDEF("metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/token"),
.type = httpProtocolTypeHttp);
driver->authClient = httpClientNew(
sckClientNew(httpUrlHost(driver->authUrl), httpUrlPort(driver->authUrl), timeout, timeout), timeout);
this->authClient = httpClientNew(
sckClientNew(httpUrlHost(this->authUrl), httpUrlPort(this->authUrl), timeout, timeout), timeout);

break;
}
Expand All @@ -1004,52 +1004,50 @@ storageGcsNew(
case storageGcsKeyTypeService:
{
KeyValue *kvKey = varKv(jsonToVar(strNewBuf(storageGetP(storageNewReadP(storagePosixNewP(FSLASH_STR), key)))));
driver->credential = varStr(kvGet(kvKey, GCS_JSON_CLIENT_EMAIL_VAR));
driver->privateKey = varStr(kvGet(kvKey, GCS_JSON_PRIVATE_KEY_VAR));
this->credential = varStr(kvGet(kvKey, GCS_JSON_CLIENT_EMAIL_VAR));
this->privateKey = varStr(kvGet(kvKey, GCS_JSON_PRIVATE_KEY_VAR));
const String *const uri = varStr(kvGet(kvKey, GCS_JSON_TOKEN_URI_VAR));

CHECK(FormatError, driver->credential != NULL && driver->privateKey != NULL && uri != NULL, "credentials missing");
CHECK(FormatError, this->credential != NULL && this->privateKey != NULL && uri != NULL, "credentials missing");

driver->authUrl = httpUrlNewParseP(uri, .type = httpProtocolTypeHttps);
this->authUrl = httpUrlNewParseP(uri, .type = httpProtocolTypeHttps);

driver->authClient = httpClientNew(
this->authClient = httpClientNew(
tlsClientNewP(
sckClientNew(httpUrlHost(driver->authUrl), httpUrlPort(driver->authUrl), timeout, timeout),
httpUrlHost(driver->authUrl), timeout, timeout, verifyPeer, .caFile = caFile, .caPath = caPath),
sckClientNew(httpUrlHost(this->authUrl), httpUrlPort(this->authUrl), timeout, timeout),
httpUrlHost(this->authUrl), timeout, timeout, verifyPeer, .caFile = caFile, .caPath = caPath),
timeout);

break;
}

// Store the authentication token
case storageGcsKeyTypeToken:
driver->token = strDup(key);
this->token = strDup(key);
break;
}

// Parse the endpoint to extract the host and port
HttpUrl *url = httpUrlNewParseP(endpoint, .type = httpProtocolTypeHttps);
driver->endpoint = httpUrlHost(url);
this->endpoint = httpUrlHost(url);

// Create the http client used to service requests
driver->httpClient = httpClientNew(
this->httpClient = httpClientNew(
tlsClientNewP(
sckClientNew(driver->endpoint, httpUrlPort(url), timeout, timeout), driver->endpoint, timeout, timeout, verifyPeer,
sckClientNew(this->endpoint, httpUrlPort(url), timeout, timeout), this->endpoint, timeout, timeout, verifyPeer,
.caFile = caFile, .caPath = caPath),
timeout);

// Create list of redacted headers
driver->headerRedactList = strLstNew();
strLstAdd(driver->headerRedactList, HTTP_HEADER_AUTHORIZATION_STR);
strLstAdd(driver->headerRedactList, GCS_HEADER_UPLOAD_ID_STR);
this->headerRedactList = strLstNew();
strLstAdd(this->headerRedactList, HTTP_HEADER_AUTHORIZATION_STR);
strLstAdd(this->headerRedactList, GCS_HEADER_UPLOAD_ID_STR);

// Create list of redacted query keys
driver->queryRedactList = strLstNew();
strLstAdd(driver->queryRedactList, GCS_QUERY_UPLOAD_ID_STR);

this = storageNew(STORAGE_GCS_TYPE, path, 0, 0, write, pathExpressionFunction, driver, driver->interface);
this->queryRedactList = strLstNew();
strLstAdd(this->queryRedactList, GCS_QUERY_UPLOAD_ID_STR);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE, this);
FUNCTION_LOG_RETURN(STORAGE, storageNew(STORAGE_GCS_TYPE, path, 0, 0, write, pathExpressionFunction, this, this->interface));
}
14 changes: 6 additions & 8 deletions src/storage/gcs/write.c
View file
Expand Up @@ -321,7 +321,7 @@ storageWriteGcsClose(THIS_VOID)

/**********************************************************************************************************************************/
FN_EXTERN StorageWrite *
storageWriteGcsNew(StorageGcs *storage, const String *name, size_t chunkSize)
storageWriteGcsNew(StorageGcs *const storage, const String *const name, const size_t chunkSize)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(STORAGE_GCS, storage);
Expand All @@ -332,13 +332,13 @@ storageWriteGcsNew(StorageGcs *storage, const String *name, size_t chunkSize)
ASSERT(storage != NULL);
ASSERT(name != NULL);

StorageWrite *this = NULL;
StorageWriteGcs *this = NULL;

OBJ_NEW_BEGIN(StorageWriteGcs, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
OBJ_NEW_BEGIN(StorageWriteGcs, .childQty = MEM_CONTEXT_QTY_MAX)
{
StorageWriteGcs *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), StorageWrite::StorageWriteGcs);
this = OBJ_NEW_ALLOC();

*driver = (StorageWriteGcs)
*this = (StorageWriteGcs)
{
.storage = storage,
.chunkSize = chunkSize,
Expand All @@ -361,10 +361,8 @@ storageWriteGcsNew(StorageGcs *storage, const String *name, size_t chunkSize)
},
},
};

this = storageWriteNew(driver, &driver->interface);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE_WRITE, this);
FUNCTION_LOG_RETURN(STORAGE_WRITE, storageWriteNew(this, &this->interface));
}
10 changes: 4 additions & 6 deletions src/storage/posix/read.c
View file
Expand Up @@ -220,13 +220,13 @@ storageReadPosixNew(

ASSERT(name != NULL);

StorageRead *this = NULL;
StorageReadPosix *this = NULL;

OBJ_NEW_BEGIN(StorageReadPosix, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX, .callbackQty = 1)
{
StorageReadPosix *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), StorageRead::StorageReadPosix);
this = OBJ_NEW_ALLOC();

*driver = (StorageReadPosix)
*this = (StorageReadPosix)
{
.storage = storage,
.fd = -1,
Expand Down Expand Up @@ -254,10 +254,8 @@ storageReadPosixNew(
},
},
};

this = storageReadNew(driver, &driver->interface);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE_READ, this);
FUNCTION_LOG_RETURN(STORAGE_READ, storageReadNew(this, &this->interface));
}
20 changes: 9 additions & 11 deletions src/storage/posix/storage.c
View file
Expand Up @@ -590,8 +590,8 @@ static const StorageInterface storageInterfacePosix =

FN_EXTERN Storage *
storagePosixNewInternal(
StringId type, const String *path, mode_t modeFile, mode_t modePath, bool write,
StoragePathExpressionCallback pathExpressionFunction, bool pathSync)
const StringId type, const String *const path, const mode_t modeFile, const mode_t modePath, const bool write,
StoragePathExpressionCallback pathExpressionFunction, const bool pathSync)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(STRING_ID, type);
Expand All @@ -612,32 +612,30 @@ storagePosixNewInternal(
userInit();

// Create the object
Storage *this = NULL;
StoragePosix *this = NULL;

OBJ_NEW_BEGIN(StoragePosix, .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
OBJ_NEW_BEGIN(StoragePosix, .childQty = MEM_CONTEXT_QTY_MAX)
{
StoragePosix *const driver = OBJ_NAME(OBJ_NEW_ALLOC(), Storage::StoragePosix);
this = OBJ_NEW_ALLOC();

*driver = (StoragePosix)
*this = (StoragePosix)
{
.interface = storageInterfacePosix,
};

// Disable path sync when not supported
if (!pathSync)
driver->interface.pathSync = NULL;
this->interface.pathSync = NULL;

// If this is a posix driver then add link features
if (type == STORAGE_POSIX_TYPE)
driver->interface.feature |=
this->interface.feature |=
1 << storageFeatureHardLink | 1 << storageFeatureSymLink | 1 << storageFeaturePathSync |
1 << storageFeatureInfoDetail;

this = storageNew(type, path, modeFile, modePath, write, pathExpressionFunction, driver, driver->interface);
}
OBJ_NEW_END();

FUNCTION_LOG_RETURN(STORAGE, this);
FUNCTION_LOG_RETURN(STORAGE, storageNew(type, path, modeFile, modePath, write, pathExpressionFunction, this, this->interface));
}

FN_EXTERN Storage *
Expand Down

0 comments on commit f6e3073

Please sign in to comment.