Skip to content

TS-4295: Add Priority Queue in lib/ts/ #537

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
Closed
masaori335 wants to merge 1 commit into from
Closed

TS-4295: Add Priority Queue in lib/ts/ #537

masaori335 wants to merge 1 commit into from

Conversation

@masaori335
Copy link
Contributor

@masaori335 masaori335 commented Mar 24, 2016

@zwoop
Copy link
Contributor

zwoop commented Mar 24, 2016

This looks clean to me, but I haven't not checked for correctness. +1 from me.

maskit
maskit reviewed Mar 30, 2016
View reviewed changes
lib/ts/PriorityQueue.h Outdated
@@ -0,0 +1,191 @@
/** @file

Priority Queue Implimentation using Min Heap.
Copy link
Member

@maskit maskit Mar 30, 2016

Choose a reason for hiding this comment

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

Implimentation -> Implementation

@maskit
Copy link
Member

maskit commented Mar 30, 2016

There should be a test case for decreasing weights.

@masaori335
Copy link
Contributor Author

masaori335 commented Mar 31, 2016

@maskit Please take another look.

maskit
maskit reviewed Mar 31, 2016
View reviewed changes
lib/ts/PriorityQueue.h Outdated
#ifndef __PRIORITY_QUEUE_H__
#define __PRIORITY_QUEUE_H__

#include "ts/Diags.h"
Copy link
Member

@maskit maskit Mar 31, 2016

Choose a reason for hiding this comment

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

What does require Diags.h?

Copy link
Contributor Author

@masaori335 masaori335 Mar 31, 2016

Choose a reason for hiding this comment

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

Nothing. I'll remove this. ( Debug logs were somewhere but I removed them. )

@maskit
Copy link
Member

maskit commented Mar 31, 2016

Looks good except the comparator part.

@masaori335
Copy link
Contributor Author

masaori335 commented Apr 6, 2016

@maskit Comparator is added.

@maskit
Copy link
Member

maskit commented Apr 6, 2016

Cool! +1

@jpeach
Copy link
Contributor

jpeach commented Apr 6, 2016

@masaori335 you can use TestBox.h for a simple regression test harness. There's other examples of doing this in the tree. The test output is better and you get a more structured set of test cases.

@masaori335
Copy link
Contributor Author

masaori335 commented Apr 6, 2016

@jpeach I agree with using many asserts is ugly. But if I understand correctly, when we use TestBox.h, we need to write tests as Regression Tests. Right?
Because of PriorityQueue is a basic data structure, I'd like to keep tests as Unit Tests.

@masaori335
Copy link
Contributor Author

masaori335 commented Apr 6, 2016

@jpeach I was wrong. I didn't know we can use TestBox.h in Unit Tests like this:p

trafficserver/lib/ts/test_X509HostnameValidator.cc

Line 190 in edf9ff1

RegressionTest::run();

I'll fix tests to use it.

@asfgit asfgit closed this in 5952dd7 Apr 8, 2016
shinrich pushed a commit to shinrich/trafficserver that referenced this pull request Sep 3, 2021
@bneradt
Ensure TS_VCONN_CLOSE_HOOK hook is called during TS_EVENT_VCONN_CLOSE. (
1078d9d 
apache#7913) (apache#537)

This causes the VCONN_CLOSE hooks to be called more consistently in the
event of TS_EVENT_VCONN_CLOSE.  Without this patch it got missed under
some circumstances, causing situations where plugins would receive a
TS_EVENT_VCONN_START but never a TS_EVENT_VCONN_CLOSE for some
connections.

(cherry picked from commit a916044)
masaori335 added a commit to masaori335/trafficserver that referenced this pull request Jul 24, 2023
@masaori335
TSan: Make Thread::cur_time thread local (apache#9184) (apache#537)
d93ac0e 
(cherry picked from commit 72e6602)

Co-authored-by: Masaori Koshiba <masaori@apache.org>
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Jul 24, 2023
@moonchen
Revert to April build, and fix ResourceManager reconfigure (apache#553)
e594d85 
* Revert "Gauge to Counter metrics for CDN Harmony (apache#544)"

This reverts commit 1a83589.

* Revert "[9.2.x] Improve performance of finding SNI Actions (apache#541)"

This reverts commit 21cbfc0.

* Revert "TSan: Make Thread::cur_time thread local (apache#9184) (apache#537)"

This reverts commit d93ac0e.

* Revert "Fix a crash in FetchSM (apache#546)"

This reverts commit e03b829.

* Revert "Remove dependency on OpenSSL's OCSP API (apache#538)"

This reverts commit 00ad803.

* Revert "Fix crashes on OCSP requests (apache#543)"

This reverts commit bbb41b9.

* Revert "Add OTHER to cipher TLS cipher metrics (apache#542)"

This reverts commit 74dbd97.

* Revert "rio: use bazinga-boringssl 19.x for boringssl build (apache#540)"

This reverts commit 9c57330.

* Revert "Use FetchSM for OCSP HTTP requests (apache#535)"

This reverts commit dbd0ec7.

* Do not schedule reconfigure event on schedule_imm_local

Co-authored-by: Mo Chen <mochen@apache.org>
Co-authored-by: Serris Lew <lserris@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@masaori335 @zwoop @maskit @jpeach