Skip to content

Create integerparsing init for substring #36413

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

Closed
wants to merge 1 commit into from

Conversation

JeaSungLEE
Copy link

@JeaSungLEE JeaSungLEE commented Mar 12, 2021

When parsing a substring, _parseASCIISlowPath is being executed. Higher performance can be expected just by wrapping it with a String once.

Note
Current speed difference

Int(substring):: Time: 0.024948954582214355
Int(String(substring)):: Time: 7.700920104980469e-05

When parsing a substring, _parseASCIISlowPath is being executed. Higher performance can be expected just by wrapping it with a String once.
@JeaSungLEE
Copy link
Author

@swift-ci Please smoke test

Copy link
Collaborator

@xwu xwu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Some notes below.

@@ -197,4 +197,31 @@ extension FixedWidthInteger {
public init?(_ description: String) {
self.init(description, radix: 10)
}

/// Creates a new integer value from the given Substring.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Creates a new integer value from the given Substring.
/// Creates a new integer value from the given `Substring`.


/// Creates a new integer value from the given Substring.
///
/// Substring can be parsed at a high speed just by substituting strings.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Substring can be parsed at a high speed just by substituting strings.

This is not relevant to the user-facing documentation.

///
/// Substring can be parsed at a high speed just by substituting strings.
///
/// The Substring passed as `description` may begin with a plus or minus sign
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The Substring passed as `description` may begin with a plus or minus sign
/// The `Substring` passed as `description` may begin with a plus or minus sign

/// Int("10000000000000000000000000") // Out of range
///
/// - Parameter description: The ASCII representation of a number.
@inlinable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs availability annotations for the standard library, and probably @_alwaysEmitIntoClient for ABI reasons. I forget exactly, but I think that implies @inlinable.

@xwu
Copy link
Collaborator

xwu commented Mar 14, 2021

cc @airspeedswift

@xwu
Copy link
Collaborator

xwu commented Apr 2, 2021

Hi @JeaSungLEE, I’ve reworked integer parsing in #36623 so that this should not be necessary; that PR will likely be merged soon—do you want to see if that fixes your issue without adding a new initializer?

@JeaSungLEE JeaSungLEE closed this Apr 3, 2021
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

Successfully merging this pull request may close these issues.

2 participants