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.

Sign up for GitHub

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

Jump to bottom

Fixes #1268 and a bug in the values output by VALUESPLIT #1269

Merged
merged 5 commits into from
Jun 30, 2023
Merged

Fixes #1268 and a bug in the values output by VALUESPLIT #1269

merged 5 commits into from
Jun 30, 2023

Conversation

hbs
Copy link
Collaborator

@hbs hbs commented Jun 26, 2023

No description provided.

@hbs hbs requested review from stggn and pi-r-p June 26, 2023 12:35
pi-r-p
pi-r-p suggested changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@pi-r-p pi-r-p left a comment

Choose a reason for hiding this comment

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

This fixes VALUESORT, but this is not efficient for VALUESPLIT.

Splitting a boolean gts do not need to sort input. Just create two empty gts, iterate on each input value to populate these gts, then keep non empty ones.

test code:

NEWGTS
1 1e6 TOLONG <%
  'v' STORE
  [ $v true ] ADDVALUE
%> FOR 
[ NOW false ] ADDVALUE
'input' STORE

'split' CHRONOSTART
$input 'x' VALUESPLIT DROP 
'split' CHRONOEND 

CHRONOSTATS

(should take 1 second, not 20)

@hbs
Copy link
Collaborator Author

hbs commented Jun 26, 2023

PTAL

pi-r-p
pi-r-p suggested changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@pi-r-p pi-r-p left a comment

Choose a reason for hiding this comment

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

60ms instead of 28s.
Sorting booleans can be done with the same strategy/code: fast boolean split then merge both. booleanSplit can be a new method in GTSHelper, used by both functions.

@pi-r-p
Copy link
Contributor

pi-r-p commented Jun 27, 2023 

[
  $input false ==
  $input true ==
] MERGE

takes 150ms (one million value input) (28s for VALUESORT)

@hbs
Copy link
Collaborator Author

hbs commented Jun 27, 2023

sort is stable, MERGE is not

@hbs hbs merged commit daab359 into senx:3.x Jun 30, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stggn stggn stggn approved these changes

@pi-r-p pi-r-p pi-r-p approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hbs @pi-r-p @stggn