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

Performance: Fix OM hierarchy to use strings for relative paths instead of URI #1617

Merged
merged 17 commits into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/CosmosClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ namespace Microsoft.Azure.Cosmos
/// <seealso href="https://docs.microsoft.com/azure/cosmos-db/request-units">Request Units</seealso>
public class CosmosClient : IDisposable
{
private readonly Uri DatabaseRootUri = new Uri(Paths.Databases_Root, UriKind.Relative);
private readonly string DatabaseRootUri = Paths.Databases_Root;
private ConsistencyLevel? accountConsistencyLevel;
private bool isDisposed = false;

Expand Down
12 changes: 6 additions & 6 deletions Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public override async Task<ResponseMessage> SendAsync(
}

public virtual async Task<T> SendAsync<T>(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
RequestOptions requestOptions,
Expand All @@ -79,7 +79,7 @@ public virtual async Task<T> SendAsync<T>(
}

ResponseMessage responseMessage = await this.SendAsync(
resourceUri: resourceUri,
resourceUriString: resourceUri,
resourceType: resourceType,
operationType: operationType,
requestOptions: requestOptions,
Expand All @@ -94,7 +94,7 @@ public virtual async Task<T> SendAsync<T>(
}

public virtual async Task<ResponseMessage> SendAsync(
Uri resourceUri,
string resourceUriString,
ResourceType resourceType,
OperationType operationType,
RequestOptions requestOptions,
Expand All @@ -105,9 +105,9 @@ public virtual async Task<ResponseMessage> SendAsync(
CosmosDiagnosticsContext diagnosticsContext,
CancellationToken cancellationToken)
{
if (resourceUri == null)
if (resourceUriString == null)
{
throw new ArgumentNullException(nameof(resourceUri));
throw new ArgumentNullException(nameof(resourceUriString));
}

// DEVNOTE: Non-Item operations need to be refactored to always pass
Expand All @@ -126,7 +126,7 @@ public virtual async Task<ResponseMessage> SendAsync(
HttpMethod method = RequestInvokerHandler.GetHttpMethod(operationType);
RequestMessage request = new RequestMessage(
method,
resourceUri,
resourceUriString,
diagnosticsContext)
{
OperationType = operationType,
Expand Down
28 changes: 22 additions & 6 deletions Microsoft.Azure.Cosmos/src/Handler/RequestMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,24 @@ public RequestMessage()
public RequestMessage(HttpMethod method, Uri requestUri)
{
this.Method = method;
this.RequestUri = requestUri;
this.RequestUriString = requestUri?.OriginalString;
this.InternalRequestUri = requestUri;
this.DiagnosticsContext = new CosmosDiagnosticsContextCore();
}

/// <summary>
/// Create a <see cref="RequestMessage"/>
/// </summary>
/// <param name="method">The http method</param>
/// <param name="requestUri">The requested URI</param>
/// <param name="requestUriString">The requested URI</param>
/// <param name="diagnosticsContext">The diagnostics object used to track the request</param>
internal RequestMessage(
HttpMethod method,
Uri requestUri,
string requestUriString,
CosmosDiagnosticsContext diagnosticsContext)
{
this.Method = method;
this.RequestUri = requestUri;
this.RequestUriString = requestUriString;
this.DiagnosticsContext = diagnosticsContext ?? throw new ArgumentNullException(nameof(diagnosticsContext));
}

Expand All @@ -70,7 +71,18 @@ internal RequestMessage(
/// <summary>
/// Gets the <see cref="Uri"/> for the current request.
/// </summary>
public virtual Uri RequestUri { get; private set; }
public virtual Uri RequestUri
{
get
{
if (this.InternalRequestUri == null)
{
this.InternalRequestUri = new Uri(this.RequestUriString, UriKind.Relative);
}

return this.InternalRequestUri;
}
}

/// <summary>
/// Gets the current <see cref="RequestMessage"/> HTTP headers.
Expand All @@ -90,6 +102,10 @@ public virtual Stream Content
}
}

internal string RequestUriString { get; private set; }
kirankumarkolli marked this conversation as resolved.
Show resolved Hide resolved

internal Uri InternalRequestUri { get; private set; }

internal CosmosDiagnosticsContext DiagnosticsContext { get; }

internal RequestOptions RequestOptions { get; set; }
Expand Down Expand Up @@ -237,7 +253,7 @@ internal DocumentServiceRequest ToDocumentServiceRequest()
}
else
{
serviceRequest = new DocumentServiceRequest(this.OperationType, this.ResourceType, this.RequestUri?.ToString(), this.Content, AuthorizationTokenType.PrimaryMasterKey, this.Headers.CosmosMessageHeaders);
serviceRequest = new DocumentServiceRequest(this.OperationType, this.ResourceType, this.RequestUriString, this.Content, AuthorizationTokenType.PrimaryMasterKey, this.Headers.CosmosMessageHeaders);
}

if (this.UseGatewayMode.HasValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public static async Task<TryCatch<CosmosQueryExecutionContext>> TryCreateFromPar

List<Documents.PartitionKeyRange> targetRanges = await CosmosQueryExecutionContextFactory.GetTargetPartitionKeyRangesAsync(
cosmosQueryContext.QueryClient,
cosmosQueryContext.ResourceLink.OriginalString,
cosmosQueryContext.ResourceLink,
partitionedQueryExecutionInfo,
containerQueryProperties,
inputParameters.Properties,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public override async Task<QueryResponseCore> ExecuteNextAsync(CancellationToken
!this.alreadyRetried)
{
await this.cosmosQueryContext.QueryClient.ForceRefreshCollectionCacheAsync(
this.cosmosQueryContext.ResourceLink.OriginalString,
this.cosmosQueryContext.ResourceLink,
cancellationToken);
this.alreadyRetried = true;
this.currentCosmosQueryExecutionContext.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal abstract class CosmosQueryClient
public abstract Action<IQueryable> OnExecuteScalarQueryCallback { get; }

public abstract Task<ContainerQueryProperties> GetCachedContainerQueryPropertiesAsync(
Uri containerLink,
string containerLink,
PartitionKey? partitionKey,
CancellationToken cancellationToken);

Expand All @@ -44,7 +44,7 @@ public abstract Task<TryCatch<PartitionedQueryExecutionInfo>> TryGetPartitionedQ
CancellationToken cancellationToken);

public abstract Task<QueryResponseCore> ExecuteItemQueryAsync(
Uri resourceUri,
string resourceUri,
Documents.ResourceType resourceType,
Documents.OperationType operationType,
Guid clientQueryCorrelationId,
Expand All @@ -58,7 +58,7 @@ public abstract Task<QueryResponseCore> ExecuteItemQueryAsync(
CancellationToken cancellationToken);

public abstract Task<PartitionedQueryExecutionInfo> ExecuteQueryPlanRequestAsync(
Uri resourceUri,
string resourceUri,
Documents.ResourceType resourceType,
Documents.OperationType operationType,
SqlQuerySpec sqlQuerySpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal abstract class CosmosQueryContext
public virtual Type ResourceType { get; }
public virtual bool IsContinuationExpected { get; }
public virtual bool AllowNonValueAggregateQuery { get; }
public virtual Uri ResourceLink { get; }
public virtual string ResourceLink { get; }
public virtual string ContainerResourceId { get; set; }
public virtual Guid CorrelatedActivityId { get; }

Expand All @@ -34,7 +34,7 @@ public CosmosQueryContext(
ResourceType resourceTypeEnum,
OperationType operationType,
Type resourceType,
Uri resourceLink,
string resourceLink,
Guid correlatedActivityId,
bool isContinuationExpected,
bool allowNonValueAggregateQuery,
Expand Down Expand Up @@ -62,7 +62,7 @@ internal abstract Task<QueryResponseCore> ExecuteQueryAsync(
CancellationToken cancellationToken);

internal abstract Task<PartitionedQueryExecutionInfo> ExecuteQueryPlanRequestAsync(
Uri resourceUri,
string resourceUri,
Documents.ResourceType resourceType,
Documents.OperationType operationType,
SqlQuerySpec sqlQuerySpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public static async Task<PartitionedQueryExecutionInfo> GetQueryPlanWithServiceI
public static Task<PartitionedQueryExecutionInfo> GetQueryPlanThroughGatewayAsync(
CosmosQueryContext queryContext,
SqlQuerySpec sqlQuerySpec,
Uri resourceLink,
string resourceLink,
PartitionKey? partitionKey,
CancellationToken cancellationToken = default)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ public CosmosQueryClientCore(
public override Action<IQueryable> OnExecuteScalarQueryCallback => this.documentClient.OnExecuteScalarQueryCallback;

public override async Task<ContainerQueryProperties> GetCachedContainerQueryPropertiesAsync(
Uri containerLink,
string containerLink,
PartitionKey? partitionKey,
CancellationToken cancellationToken)
{
ContainerProperties containerProperties = await this.clientContext.GetCachedContainerPropertiesAsync(
containerLink.OriginalString,
containerLink,
cancellationToken);

string effectivePartitionKeyString = null;
Expand Down Expand Up @@ -116,7 +116,7 @@ public override async Task<TryCatch<PartitionedQueryExecutionInfo>> TryGetPartit
}

public override async Task<QueryResponseCore> ExecuteItemQueryAsync(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
Guid clientQueryCorrelationId,
Expand Down Expand Up @@ -164,7 +164,7 @@ public override async Task<QueryResponseCore> ExecuteItemQueryAsync(
}

public override async Task<PartitionedQueryExecutionInfo> ExecuteQueryPlanRequestAsync(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
SqlQuerySpec sqlQuerySpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public CosmosQueryContextCore(
ResourceType resourceTypeEnum,
OperationType operationType,
Type resourceType,
Uri resourceLink,
string resourceLink,
Guid correlatedActivityId,
bool isContinuationExpected,
bool allowNonValueAggregateQuery,
Expand Down Expand Up @@ -92,7 +92,7 @@ internal override Task<QueryResponseCore> ExecuteQueryAsync(
}

internal override Task<PartitionedQueryExecutionInfo> ExecuteQueryPlanRequestAsync(
Uri resourceUri,
string resourceUri,
Documents.ResourceType resourceType,
Documents.OperationType operationType,
SqlQuerySpec sqlQuerySpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static QueryIterator Create(
string continuationToken,
FeedRangeInternal feedRangeInternal,
QueryRequestOptions queryRequestOptions,
Uri resourceLink,
string resourceLink,
bool isContinuationExpected,
bool allowNonValueAggregateQuery,
bool forcePassthrough,
Expand Down
16 changes: 9 additions & 7 deletions Microsoft.Azure.Cosmos/src/Resource/ClientContextCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ internal static CosmosClientContext Create(
/// <param name="uriPathSegment">The URI path segment</param>
/// <param name="id">The id of the resource</param>
/// <returns>A resource link in the format of {parentLink}/this.UriPathSegment/this.Name with this.Name being a Uri escaped version</returns>
internal override Uri CreateLink(
internal override string CreateLink(
string parentLink,
string uriPathSegment,
string id)
Expand All @@ -160,6 +160,8 @@ internal override Uri CreateLink(
int parentLinkLength = parentLink?.Length ?? 0;
string idUriEscaped = Uri.EscapeUriString(id);

Debug.Assert(parentLinkLength == 0 || !parentLink.EndsWith("/"));

StringBuilder stringBuilder = new StringBuilder(parentLinkLength + 2 + uriPathSegment.Length + idUriEscaped.Length);
if (parentLinkLength > 0)
{
Expand All @@ -170,7 +172,7 @@ internal override Uri CreateLink(
stringBuilder.Append(uriPathSegment);
stringBuilder.Append("/");
stringBuilder.Append(idUriEscaped);
return new Uri(stringBuilder.ToString(), UriKind.Relative);
kirankumarkolli marked this conversation as resolved.
Show resolved Hide resolved
return stringBuilder.ToString();
}

internal override void ValidateResource(string resourceId)
Expand All @@ -180,7 +182,7 @@ internal override void ValidateResource(string resourceId)
}

internal override Task<ResponseMessage> ProcessResourceOperationStreamAsync(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
RequestOptions requestOptions,
Expand Down Expand Up @@ -232,7 +234,7 @@ internal override Task<ResponseMessage> ProcessResourceOperationStreamAsync(
}

internal override Task<ResponseMessage> ProcessResourceOperationStreamAsync(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
RequestOptions requestOptions,
Expand All @@ -245,7 +247,7 @@ internal override Task<ResponseMessage> ProcessResourceOperationStreamAsync(
{
this.ThrowIfDisposed();
return this.RequestHandler.SendAsync(
resourceUri: resourceUri,
resourceUriString: resourceUri,
resourceType: resourceType,
operationType: operationType,
requestOptions: requestOptions,
Expand All @@ -258,7 +260,7 @@ internal override Task<ResponseMessage> ProcessResourceOperationStreamAsync(
}

internal override Task<T> ProcessResourceOperationAsync<T>(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
RequestOptions requestOptions,
Expand Down Expand Up @@ -344,7 +346,7 @@ protected virtual void Dispose(bool disposing)
}

private async Task<ResponseMessage> ProcessResourceOperationAsBulkStreamAsync(
Uri resourceUri,
string resourceUri,
ResourceType resourceType,
OperationType operationType,
RequestOptions requestOptions,
Expand Down
14 changes: 7 additions & 7 deletions Microsoft.Azure.Cosmos/src/Resource/Conflict/ConflictsCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public override Task<ResponseMessage> DeleteAsync(
throw new ArgumentNullException(nameof(conflict));
}

Uri conflictLink = this.clientContext.CreateLink(
parentLink: this.container.LinkUri.OriginalString,
string conflictLink = this.clientContext.CreateLink(
parentLink: this.container.LinkUri,
uriPathSegment: Paths.ConflictsPathSegment,
id: conflict.Id);

Expand Down Expand Up @@ -146,18 +146,18 @@ public override async Task<ItemResponse<T>> ReadCurrentAsync<T>(
string databaseResourceId = await databaseCore.GetRIDAsync(cancellationToken);
string containerResourceId = await this.container.GetRIDAsync(cancellationToken);

Uri dbLink = this.clientContext.CreateLink(
string dbLink = this.clientContext.CreateLink(
parentLink: string.Empty,
uriPathSegment: Paths.DatabasesPathSegment,
id: databaseResourceId);

Uri containerLink = this.clientContext.CreateLink(
parentLink: dbLink.OriginalString,
string containerLink = this.clientContext.CreateLink(
parentLink: dbLink,
uriPathSegment: Paths.CollectionsPathSegment,
id: containerResourceId);

Uri itemLink = this.clientContext.CreateLink(
parentLink: containerLink.OriginalString,
string itemLink = this.clientContext.CreateLink(
parentLink: containerLink,
uriPathSegment: Paths.DocumentsPathSegment,
id: cosmosConflict.SourceResourceId);

Expand Down
Loading